Skip to content

Commit 4ca2def

Browse files
lichuanglichuang
authored
feat: add create or replace password policy support (#14654)
1 parent aa49e5f commit 4ca2def

File tree

10 files changed

+151
-55
lines changed

10 files changed

+151
-55
lines changed

‎src/query/ast/src/ast/statements/password_policy.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,26 @@
1515
use std::fmt::Display;
1616
use std::fmt::Formatter;
1717

18+
use databend_common_meta_app::schema::CreateOption;
19+
1820
#[derive(Debug, Clone, PartialEq)]
1921
pub struct CreatePasswordPolicyStmt {
20-
pub if_not_exists: bool,
22+
pub create_option: CreateOption,
2123
pub name: String,
2224
pub set_options: PasswordSetOptions,
2325
}
2426

2527
impl Display for CreatePasswordPolicyStmt {
2628
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
27-
write!(f, "CREATE PASSWORD POLICY ")?;
28-
if self.if_not_exists {
29-
write!(f, "IF NOT EXISTS ")?;
29+
write!(f, "CREATE ")?;
30+
if let CreateOption::CreateOrReplace = self.create_option {
31+
write!(f, "OR REPLACE ")?;
32+
}
33+
write!(f, "PASSWORD POLICY ")?;
34+
if let CreateOption::CreateIfNotExists(if_not_exists) = self.create_option {
35+
if if_not_exists {
36+
write!(f, "IF NOT EXISTS ")?;
37+
}
3038
}
3139
write!(f, "{}", self.name)?;
3240
write!(f, "{}", self.set_options)?;

‎src/query/ast/src/parser/statement.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,18 +1718,20 @@ pub fn statement(i: Input) -> IResult<StatementWithFormat> {
17181718
rule! { SHOW ~ NETWORK ~ ^POLICIES },
17191719
);
17201720

1721-
let create_password_policy = map(
1721+
let create_password_policy = map_res(
17221722
rule! {
1723-
CREATE ~ PASSWORD ~ ^POLICY ~ ( IF ~ ^NOT ~ ^EXISTS )? ~ ^#ident
1723+
CREATE ~ (OR ~ REPLACE)? ~ PASSWORD ~ ^POLICY ~ ( IF ~ ^NOT ~ ^EXISTS )? ~ ^#ident
17241724
~ #password_set_options
17251725
},
1726-
|(_, _, _, opt_if_not_exists, name, set_options)| {
1726+
|(_, opt_or_replace, _, _, opt_if_not_exists, name, set_options)| {
1727+
let create_option =
1728+
parse_create_option(opt_or_replace.is_some(), opt_if_not_exists.is_some())?;
17271729
let stmt = CreatePasswordPolicyStmt {
1728-
if_not_exists: opt_if_not_exists.is_some(),
1730+
create_option,
17291731
name: name.to_string(),
17301732
set_options,
17311733
};
1732-
Statement::CreatePasswordPolicy(stmt)
1734+
Ok(Statement::CreatePasswordPolicy(stmt))
17331735
},
17341736
);
17351737
let alter_password_policy = map(

‎src/query/management/src/password_policy/password_policy_api.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,17 @@
1414

1515
use databend_common_exception::Result;
1616
use databend_common_meta_app::principal::PasswordPolicy;
17+
use databend_common_meta_app::schema::CreateOption;
1718
use databend_common_meta_types::MatchSeq;
1819
use databend_common_meta_types::SeqV;
1920

2021
#[async_trait::async_trait]
2122
pub trait PasswordPolicyApi: Sync + Send {
22-
async fn add_password_policy(&self, password_policy: PasswordPolicy) -> Result<u64>;
23+
async fn add_password_policy(
24+
&self,
25+
password_policy: PasswordPolicy,
26+
create_option: &CreateOption,
27+
) -> Result<()>;
2328

2429
async fn update_password_policy(
2530
&self,

‎src/query/management/src/password_policy/password_policy_mgr.rs

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use databend_common_base::base::escape_for_key;
1818
use databend_common_exception::ErrorCode;
1919
use databend_common_exception::Result;
2020
use databend_common_meta_app::principal::PasswordPolicy;
21+
use databend_common_meta_app::schema::CreateOption;
2122
use databend_common_meta_kvapi::kvapi;
2223
use databend_common_meta_kvapi::kvapi::UpsertKVReq;
2324
use databend_common_meta_types::MatchSeq;
@@ -67,8 +68,12 @@ impl PasswordPolicyMgr {
6768
impl PasswordPolicyApi for PasswordPolicyMgr {
6869
#[async_backtrace::framed]
6970
#[minitrace::trace]
70-
async fn add_password_policy(&self, password_policy: PasswordPolicy) -> Result<u64> {
71-
let match_seq = MatchSeq::Exact(0);
71+
async fn add_password_policy(
72+
&self,
73+
password_policy: PasswordPolicy,
74+
create_option: &CreateOption,
75+
) -> Result<()> {
76+
let seq = MatchSeq::from(*create_option);
7277
let key = self.make_password_policy_key(password_policy.name.as_str())?;
7378
let value = Operation::Update(serialize_struct(
7479
&password_policy,
@@ -77,16 +82,20 @@ impl PasswordPolicyApi for PasswordPolicyMgr {
7782
)?);
7883

7984
let kv_api = self.kv_api.clone();
80-
let upsert_kv = kv_api.upsert_kv(UpsertKVReq::new(&key, match_seq, value, None));
85+
let res = kv_api
86+
.upsert_kv(UpsertKVReq::new(&key, seq, value, None))
87+
.await?;
8188

82-
let res_seq = upsert_kv.await?.added_seq_or_else(|_v| {
83-
ErrorCode::PasswordPolicyAlreadyExists(format!(
84-
"Password policy '{}' already exists.",
85-
password_policy.name
86-
))
87-
})?;
89+
if let CreateOption::CreateIfNotExists(false) = create_option {
90+
if res.prev.is_some() {
91+
return Err(ErrorCode::PasswordPolicyAlreadyExists(format!(
92+
"Password policy '{}' already exists.",
93+
password_policy.name
94+
)));
95+
}
96+
}
8897

89-
Ok(res_seq)
98+
Ok(())
9099
}
91100

92101
#[async_backtrace::framed]

‎src/query/service/src/interpreters/interpreter_password_policy_create.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ impl Interpreter for CreatePasswordPolicyInterpreter {
124124
update_on: None,
125125
};
126126
user_mgr
127-
.add_password_policy(&tenant, password_policy, plan.if_not_exists)
127+
.add_password_policy(&tenant, password_policy, &plan.create_option)
128128
.await?;
129129

130130
Ok(PipelineBuildResult::create())

‎src/query/sql/src/planner/binder/ddl/password_policy.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,14 @@ impl Binder {
3232
stmt: &CreatePasswordPolicyStmt,
3333
) -> Result<Plan> {
3434
let CreatePasswordPolicyStmt {
35-
if_not_exists,
35+
create_option,
3636
name,
3737
set_options,
3838
} = stmt;
3939

4040
let tenant = self.ctx.get_tenant();
4141
let plan = CreatePasswordPolicyPlan {
42-
if_not_exists: *if_not_exists,
42+
create_option: *create_option,
4343
tenant,
4444
name: name.to_string(),
4545
set_options: set_options.clone(),

‎src/query/sql/src/planner/plans/ddl/account.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ impl ShowNetworkPoliciesPlan {
204204

205205
#[derive(Clone, Debug, PartialEq)]
206206
pub struct CreatePasswordPolicyPlan {
207-
pub if_not_exists: bool,
207+
pub create_option: CreateOption,
208208
pub tenant: String,
209209
pub name: String,
210210
pub set_options: PasswordSetOptions,

‎src/query/users/src/password_policy.rs

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use databend_common_meta_app::principal::PasswordPolicy;
2525
use databend_common_meta_app::principal::UserIdentity;
2626
use databend_common_meta_app::principal::UserInfo;
2727
use databend_common_meta_app::principal::UserOption;
28+
use databend_common_meta_app::schema::CreateOption;
2829
use databend_common_meta_types::MatchSeq;
2930
use log::info;
3031
use passwords::analyzer;
@@ -65,30 +66,14 @@ impl UserApiProvider {
6566
&self,
6667
tenant: &str,
6768
password_policy: PasswordPolicy,
68-
if_not_exists: bool,
69-
) -> Result<u64> {
69+
create_option: &CreateOption,
70+
) -> Result<()> {
7071
check_password_policy(&password_policy)?;
7172

72-
if if_not_exists
73-
&& self
74-
.exists_password_policy(tenant, password_policy.name.as_str())
75-
.await?
76-
{
77-
return Ok(0);
78-
}
79-
8073
let client = self.get_password_policy_api_client(tenant)?;
81-
let add_password_policy = client.add_password_policy(password_policy);
82-
match add_password_policy.await {
83-
Ok(res) => Ok(res),
84-
Err(e) => {
85-
if if_not_exists && e.code() == ErrorCode::PASSWORD_POLICY_ALREADY_EXISTS {
86-
Ok(0)
87-
} else {
88-
Err(e.add_message_back("(while add password policy)"))
89-
}
90-
}
91-
}
74+
client
75+
.add_password_policy(password_policy, create_option)
76+
.await
9277
}
9378

9479
// Update password policy.

‎src/query/users/tests/it/password_policy.rs

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -66,23 +66,35 @@ async fn test_password_policy() -> Result<()> {
6666
};
6767

6868
let res = user_mgr
69-
.add_password_policy(tenant, password_policy.clone(), false)
69+
.add_password_policy(
70+
tenant,
71+
password_policy.clone(),
72+
&CreateOption::CreateIfNotExists(false),
73+
)
7074
.await;
7175
assert!(res.is_ok());
7276

7377
// invalid min length
7478
let mut invalid_password_policy1 = password_policy.clone();
7579
invalid_password_policy1.min_length = 0;
7680
let res = user_mgr
77-
.add_password_policy(tenant, invalid_password_policy1, false)
81+
.add_password_policy(
82+
tenant,
83+
invalid_password_policy1,
84+
&CreateOption::CreateIfNotExists(false),
85+
)
7886
.await;
7987
assert!(res.is_err());
8088

8189
// invalid max length
8290
let mut invalid_password_policy2 = password_policy.clone();
8391
invalid_password_policy2.max_length = 260;
8492
let res = user_mgr
85-
.add_password_policy(tenant, invalid_password_policy2, false)
93+
.add_password_policy(
94+
tenant,
95+
invalid_password_policy2,
96+
&CreateOption::CreateIfNotExists(false),
97+
)
8698
.await;
8799
assert!(res.is_err());
88100

@@ -91,7 +103,11 @@ async fn test_password_policy() -> Result<()> {
91103
invalid_password_policy3.min_length = 30;
92104
invalid_password_policy3.max_length = 20;
93105
let res = user_mgr
94-
.add_password_policy(tenant, invalid_password_policy3, false)
106+
.add_password_policy(
107+
tenant,
108+
invalid_password_policy3,
109+
&CreateOption::CreateIfNotExists(false),
110+
)
95111
.await;
96112
assert!(res.is_err());
97113

@@ -102,7 +118,11 @@ async fn test_password_policy() -> Result<()> {
102118
invalid_password_policy4.min_numeric_chars = 272;
103119
invalid_password_policy4.min_special_chars = 273;
104120
let res = user_mgr
105-
.add_password_policy(tenant, invalid_password_policy4, false)
121+
.add_password_policy(
122+
tenant,
123+
invalid_password_policy4,
124+
&CreateOption::CreateIfNotExists(false),
125+
)
106126
.await;
107127
assert!(res.is_err());
108128

@@ -114,7 +134,11 @@ async fn test_password_policy() -> Result<()> {
114134
invalid_password_policy5.min_numeric_chars = 12;
115135
invalid_password_policy5.min_special_chars = 13;
116136
let res = user_mgr
117-
.add_password_policy(tenant, invalid_password_policy5, false)
137+
.add_password_policy(
138+
tenant,
139+
invalid_password_policy5,
140+
&CreateOption::CreateIfNotExists(false),
141+
)
118142
.await;
119143
assert!(res.is_err());
120144

@@ -123,31 +147,47 @@ async fn test_password_policy() -> Result<()> {
123147
invalid_password_policy6.min_age_days = 20;
124148
invalid_password_policy6.max_age_days = 10;
125149
let res = user_mgr
126-
.add_password_policy(tenant, invalid_password_policy6, false)
150+
.add_password_policy(
151+
tenant,
152+
invalid_password_policy6,
153+
&CreateOption::CreateIfNotExists(false),
154+
)
127155
.await;
128156
assert!(res.is_err());
129157

130158
// invalid max retries
131159
let mut invalid_password_policy7 = password_policy.clone();
132160
invalid_password_policy7.max_retries = 20;
133161
let res = user_mgr
134-
.add_password_policy(tenant, invalid_password_policy7, false)
162+
.add_password_policy(
163+
tenant,
164+
invalid_password_policy7,
165+
&CreateOption::CreateIfNotExists(false),
166+
)
135167
.await;
136168
assert!(res.is_err());
137169

138170
// invalid lockout time mins
139171
let mut invalid_password_policy8 = password_policy.clone();
140172
invalid_password_policy8.lockout_time_mins = 2000;
141173
let res = user_mgr
142-
.add_password_policy(tenant, invalid_password_policy8, false)
174+
.add_password_policy(
175+
tenant,
176+
invalid_password_policy8,
177+
&CreateOption::CreateIfNotExists(false),
178+
)
143179
.await;
144180
assert!(res.is_err());
145181

146182
// invalid history
147183
let mut invalid_password_policy9 = password_policy.clone();
148184
invalid_password_policy9.history = 50;
149185
let res = user_mgr
150-
.add_password_policy(tenant, invalid_password_policy9, false)
186+
.add_password_policy(
187+
tenant,
188+
invalid_password_policy9,
189+
&CreateOption::CreateIfNotExists(false),
190+
)
151191
.await;
152192
assert!(res.is_err());
153193

‎tests/sqllogictests/suites/base/05_ddl/05_0035_ddl_password_policy.test

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,3 +141,50 @@ DESC PASSWORD POLICY test_policy
141141
statement ok
142142
DROP PASSWORD POLICY default_policy
143143

144+
statement ok
145+
CREATE PASSWORD POLICY replace_policy
146+
PASSWORD_MIN_LENGTH = 12
147+
PASSWORD_MAX_LENGTH = 24
148+
PASSWORD_MIN_UPPER_CASE_CHARS = 2
149+
PASSWORD_MIN_LOWER_CASE_CHARS = 2
150+
PASSWORD_MIN_NUMERIC_CHARS = 2
151+
PASSWORD_MIN_SPECIAL_CHARS = 2
152+
PASSWORD_MIN_AGE_DAYS = 1
153+
PASSWORD_MAX_AGE_DAYS = 30
154+
PASSWORD_MAX_RETRIES = 3
155+
PASSWORD_LOCKOUT_TIME_MINS = 30
156+
PASSWORD_HISTORY = 5
157+
COMMENT = 'test comment'
158+
159+
statement error 1005
160+
CREATE OR REPLACE PASSWORD POLICY IF NOT EXISTS replace_policy
161+
PASSWORD_MIN_LENGTH = 24
162+
PASSWORD_MAX_LENGTH = 48
163+
PASSWORD_MIN_UPPER_CASE_CHARS = 2
164+
PASSWORD_MIN_LOWER_CASE_CHARS = 2
165+
PASSWORD_MIN_NUMERIC_CHARS = 2
166+
PASSWORD_MIN_SPECIAL_CHARS = 2
167+
PASSWORD_MIN_AGE_DAYS = 1
168+
PASSWORD_MAX_AGE_DAYS = 30
169+
PASSWORD_MAX_RETRIES = 3
170+
PASSWORD_LOCKOUT_TIME_MINS = 30
171+
PASSWORD_HISTORY = 5
172+
COMMENT = 'test comment'
173+
174+
statement ok
175+
CREATE OR REPLACE PASSWORD POLICY replace_policy
176+
PASSWORD_MIN_LENGTH = 24
177+
PASSWORD_MAX_LENGTH = 48
178+
PASSWORD_MIN_UPPER_CASE_CHARS = 2
179+
PASSWORD_MIN_LOWER_CASE_CHARS = 2
180+
PASSWORD_MIN_NUMERIC_CHARS = 2
181+
PASSWORD_MIN_SPECIAL_CHARS = 2
182+
PASSWORD_MIN_AGE_DAYS = 1
183+
PASSWORD_MAX_AGE_DAYS = 30
184+
PASSWORD_MAX_RETRIES = 3
185+
PASSWORD_LOCKOUT_TIME_MINS = 30
186+
PASSWORD_HISTORY = 5
187+
COMMENT = 'test comment'
188+
189+
statement ok
190+
DROP PASSWORD POLICY replace_policy

0 commit comments

Comments
 (0)