diff --git a/docs/dev/src/SUMMARY.md b/docs/dev/src/SUMMARY.md index 4c240b5cf040a..7f5fa68f8024d 100644 --- a/docs/dev/src/SUMMARY.md +++ b/docs/dev/src/SUMMARY.md @@ -46,6 +46,7 @@ - [Relational Table](./design/relational-table.md) - [Multiple Object Storage Backends](./design/multi-object-store.md) - [Meta Service](./design/meta-service.md) +- [RBAC Parser Surface](./design/rbac-parser-surface.md) - [Data Model and Encoding](./design/data-model-and-encoding.md) - [Batch Local Execution Mode](./design/batch-local-execution-mode.md) - [Consistent Hash](./design/consistent-hash.md) diff --git a/docs/dev/src/design/rbac-parser-surface.md b/docs/dev/src/design/rbac-parser-surface.md new file mode 100644 index 0000000000000..790d7aec8a437 --- /dev/null +++ b/docs/dev/src/design/rbac-parser-surface.md @@ -0,0 +1,54 @@ +# PostgreSQL-style RBAC parser surface + +This note documents the staged parser and dispatch surface that was added for +PostgreSQL-style RBAC compatibility work. + +## Scope + +The SQL parser and frontend statement dispatcher now recognize these statements: + +- `CREATE ROLE` +- `ALTER ROLE` +- role-membership `GRANT ... TO ... [WITH ADMIN OPTION]` +- role-membership `REVOKE [ADMIN OPTION FOR] ... FROM ...` +- `SET ROLE` +- `RESET ROLE` + +The parser also accepts the PostgreSQL-style role options currently needed by the +test matrix for this lane, including: + +- `CREATEDB` / `NOCREATEDB` +- `CREATEROLE` / `NOCREATEROLE` +- `INHERIT` / `NOINHERIT` +- `LOGIN` / `NOLOGIN` +- legacy RisingWave user aliases that already existed + +## Current implementation boundary + +This lane intentionally lands parser and dispatcher support before the meta and +session lanes are finished. + +That means some statements are **accepted and dispatched** but still terminate in +explicit `not implemented` frontend handlers until the downstream lanes land: + +- membership grant/revoke execution +- `SET ROLE` +- `RESET ROLE` + +This staged boundary is deliberate so that: + +1. parser and AST contracts stabilize early, +2. regression tests can be written against a fixed SQL surface, and +3. meta/session work can plug into the already-routed statement variants. + +## Verification coverage + +The worker verification for this lane covers both unit-style parser assertions and +parser-test YAML contracts: + +- `sqlparser_common` role-focused tests +- `role.yaml` +- `drop.yaml` + +These suites are the source of truth for the accepted SQL spellings and formatter +output of the parser surface described above. diff --git a/src/frontend/src/handler/alter_user.rs b/src/frontend/src/handler/alter_user.rs index aabbaf10a0112..c96251d9cca9a 100644 --- a/src/frontend/src/handler/alter_user.rs +++ b/src/frontend/src/handler/alter_user.rs @@ -119,6 +119,15 @@ fn alter_prost_user_info( user_info.is_admin = false; update_fields.insert(UpdateField::Admin); } + UserOption::CreateRole + | UserOption::NoCreateRole + | UserOption::Inherit + | UserOption::NoInherit => { + return Err(ErrorCode::InvalidParameterValue( + "role options are not supported yet".to_owned(), + ) + .into()); + } UserOption::EncryptedPassword(p) => { if !p.0.is_empty() { user_info.auth_info = encrypted_password(&user_info.name, &p.0); @@ -299,6 +308,24 @@ mod tests { ); } + #[tokio::test] + async fn test_alter_user_rejects_role_options() { + let frontend = LocalFrontend::new(Default::default()).await; + frontend + .run_sql("CREATE USER role_option_user") + .await + .unwrap(); + + for option in ["CREATEROLE", "NOCREATEROLE", "INHERIT", "NOINHERIT"] { + let err = frontend + .run_sql(&format!("ALTER USER role_option_user WITH {option}")) + .await + .unwrap_err() + .to_string(); + assert!(err.contains("role options are not supported yet"), "{err}"); + } + } + #[tokio::test] async fn test_alter_admin_user() { let frontend = LocalFrontend::new(Default::default()).await; diff --git a/src/frontend/src/handler/create_user.rs b/src/frontend/src/handler/create_user.rs index 0dfcd09cc8b4c..60afa7ea93316 100644 --- a/src/frontend/src/handler/create_user.rs +++ b/src/frontend/src/handler/create_user.rs @@ -110,6 +110,15 @@ pub async fn handle_create_user( UserOption::NoLogin => user_info.can_login = false, UserOption::Admin => user_info.is_admin = true, UserOption::NoAdmin => user_info.is_admin = false, + UserOption::CreateRole + | UserOption::NoCreateRole + | UserOption::Inherit + | UserOption::NoInherit => { + return Err(ErrorCode::InvalidParameterValue( + "role options are not supported yet".to_owned(), + ) + .into()); + } UserOption::EncryptedPassword(password) => { if !password.0.is_empty() { user_info.auth_info = encrypted_password(&user_info.name, &password.0); @@ -235,6 +244,23 @@ mod tests { ); } + #[tokio::test] + async fn test_create_user_rejects_role_options() { + let frontend = LocalFrontend::new(Default::default()).await; + + for (idx, option) in ["CREATEROLE", "NOCREATEROLE", "INHERIT", "NOINHERIT"] + .into_iter() + .enumerate() + { + let err = frontend + .run_sql(&format!("CREATE USER role_option_user_{idx} WITH {option}")) + .await + .unwrap_err() + .to_string(); + assert!(err.contains("role options are not supported yet"), "{err}"); + } + } + #[tokio::test] async fn test_create_admin_user() { let frontend = LocalFrontend::new(Default::default()).await; diff --git a/src/frontend/src/handler/mod.rs b/src/frontend/src/handler/mod.rs index b39fb8090f2d9..e1630691bd843 100644 --- a/src/frontend/src/handler/mod.rs +++ b/src/frontend/src/handler/mod.rs @@ -535,7 +535,7 @@ pub async fn handle( | ObjectType::Schema | ObjectType::Connection | ObjectType::Secret => true, - ObjectType::Database | ObjectType::User => { + ObjectType::Database | ObjectType::User | ObjectType::Role => { bail_not_implemented!("DROP CASCADE"); } } @@ -580,6 +580,9 @@ pub async fn handle( ObjectType::User => { drop_user::handle_drop_user(handler_args, object_name, if_exists).await } + ObjectType::Role => { + bail_not_implemented!("DROP ROLE") + } ObjectType::View => { drop_view::handle_drop_view(handler_args, object_name, if_exists, cascade).await } diff --git a/src/sqlparser/src/ast/mod.rs b/src/sqlparser/src/ast/mod.rs index ca20aed36773d..013e893463711 100644 --- a/src/sqlparser/src/ast/mod.rs +++ b/src/sqlparser/src/ast/mod.rs @@ -1633,6 +1633,13 @@ pub enum Statement { with_grant_option: bool, granted_by: Option, }, + /// GRANT roles TO grantees + GrantRole { + roles: Vec, + grantees: Vec, + role_options: Vec, + granted_by: Option, + }, /// REVOKE privileges ON objects FROM grantees Revoke { privileges: Privileges, @@ -1642,6 +1649,14 @@ pub enum Statement { revoke_grant_option: bool, cascade: bool, }, + /// REVOKE roles FROM grantees + RevokeRole { + roles: Vec, + grantees: Vec, + revoke_role_option: Option, + granted_by: Option, + cascade: bool, + }, /// `DEALLOCATE [ PREPARE ] { name | ALL }` /// /// Note: this is a PostgreSQL-specific statement. @@ -1683,8 +1698,19 @@ pub enum Statement { }, /// CREATE USER CreateUser(CreateUserStatement), + /// CREATE ROLE + CreateRole(CreateUserStatement), /// ALTER USER AlterUser(AlterUserStatement), + /// ALTER ROLE + AlterRole(AlterUserStatement), + /// `SET [ SESSION | LOCAL ] ROLE ` + SetRole { + context_modifier: Option, + role_name: SetRoleSpec, + }, + /// `RESET ROLE` + ResetRole, /// ALTER SYSTEM SET configuration_parameter { TO | = } { value | 'value' | DEFAULT } AlterSystem { param: Ident, @@ -2377,6 +2403,22 @@ impl Statement { } Ok(()) } + Statement::GrantRole { + roles, + grantees, + role_options, + granted_by, + } => { + write!(f, "GRANT {} ", display_comma_separated(roles))?; + write!(f, "TO {}", display_comma_separated(grantees))?; + if !role_options.is_empty() { + write!(f, " WITH {}", display_comma_separated(role_options))?; + } + if let Some(grantor) = granted_by { + write!(f, " GRANTED BY {}", grantor)?; + } + Ok(()) + } Statement::Revoke { privileges, objects, @@ -2403,6 +2445,28 @@ impl Statement { write!(f, " {}", if *cascade { "CASCADE" } else { "RESTRICT" })?; Ok(()) } + Statement::RevokeRole { + roles, + grantees, + revoke_role_option, + granted_by, + cascade, + } => { + write!( + f, + "REVOKE {}{} FROM {}", + revoke_role_option + .as_ref() + .map(|opt| format!("{opt} OPTION FOR ")) + .unwrap_or_default(), + display_comma_separated(roles), + display_comma_separated(grantees), + )?; + if let Some(grantor) = granted_by { + write!(f, " GRANTED BY {}", grantor)?; + } + write!(f, " {}", if *cascade { "CASCADE" } else { "RESTRICT" }) + } Statement::Deallocate { name, prepare } => { if let Some(name) = name { write!( @@ -2453,9 +2517,26 @@ impl Statement { Statement::CreateUser(statement) => { write!(f, "CREATE USER {}", statement) } + Statement::CreateRole(statement) => { + write!(f, "CREATE ROLE {}", statement) + } Statement::AlterUser(statement) => { write!(f, "ALTER USER {}", statement) } + Statement::AlterRole(statement) => { + write!(f, "ALTER ROLE {}", statement) + } + Statement::SetRole { + context_modifier, + role_name, + } => { + write!(f, "SET ")?; + if let Some(modifier) = context_modifier { + write!(f, "{} ", modifier)?; + } + write!(f, "ROLE {}", role_name) + } + Statement::ResetRole => write!(f, "RESET ROLE"), Statement::AlterSystem { param, value } => { f.write_str("ALTER SYSTEM SET ")?; write!(f, "{param} = {value}",) @@ -2546,6 +2627,7 @@ impl Statement { | Statement::CreateConnection { .. } | Statement::CreateSecret { .. } | Statement::CreateUser { .. } + | Statement::CreateRole { .. } | Statement::CreateDatabase { .. } | Statement::CreateFunction { .. } | Statement::CreateAggregate { .. } @@ -3201,6 +3283,7 @@ pub enum ObjectType { Source, Sink, Database, + Role, User, Connection, Secret, @@ -3218,6 +3301,7 @@ impl fmt::Display for ObjectType { ObjectType::Source => "SOURCE", ObjectType::Sink => "SINK", ObjectType::Database => "DATABASE", + ObjectType::Role => "ROLE", ObjectType::User => "USER", ObjectType::Secret => "SECRET", ObjectType::Connection => "CONNECTION", @@ -3244,6 +3328,8 @@ impl ParseTo for ObjectType { ObjectType::Schema } else if parser.parse_keyword(Keyword::DATABASE) { ObjectType::Database + } else if parser.parse_keyword(Keyword::ROLE) { + ObjectType::Role } else if parser.parse_keyword(Keyword::USER) { ObjectType::User } else if parser.parse_keyword(Keyword::CONNECTION) { @@ -3254,7 +3340,7 @@ impl ParseTo for ObjectType { ObjectType::Subscription } else { return parser.expected( - "TABLE, VIEW, INDEX, MATERIALIZED VIEW, SOURCE, SINK, SUBSCRIPTION, SCHEMA, DATABASE, USER, SECRET or CONNECTION after DROP", + "TABLE, VIEW, INDEX, MATERIALIZED VIEW, SOURCE, SINK, SUBSCRIPTION, SCHEMA, DATABASE, ROLE, USER, SECRET or CONNECTION after DROP", ); }; Ok(object_type) diff --git a/src/sqlparser/src/ast/statement.rs b/src/sqlparser/src/ast/statement.rs index 04333dd90fa81..122090f0b70cf 100644 --- a/src/sqlparser/src/ast/statement.rs +++ b/src/sqlparser/src/ast/statement.rs @@ -1022,14 +1022,81 @@ pub enum AlterUserMode { Rename(ObjectName), } +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum RoleContextModifier { + Session, + Local, +} + +impl fmt::Display for RoleContextModifier { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + RoleContextModifier::Session => write!(f, "SESSION"), + RoleContextModifier::Local => write!(f, "LOCAL"), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum SetRoleSpec { + Name(Ident), + None, +} + +impl fmt::Display for SetRoleSpec { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + SetRoleSpec::Name(name) => write!(f, "{}", name), + SetRoleSpec::None => write!(f, "NONE"), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum RoleOptionKind { + Admin, + Inherit, + Set, +} + +impl fmt::Display for RoleOptionKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + RoleOptionKind::Admin => write!(f, "ADMIN"), + RoleOptionKind::Inherit => write!(f, "INHERIT"), + RoleOptionKind::Set => write!(f, "SET"), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct RoleOptionSpec { + pub kind: RoleOptionKind, + pub value: bool, +} + +impl fmt::Display for RoleOptionSpec { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match (&self.kind, self.value) { + (RoleOptionKind::Admin, true) => write!(f, "ADMIN OPTION"), + (_, true) => write!(f, "{} TRUE", self.kind), + (_, false) => write!(f, "{} FALSE", self.kind), + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum UserOption { SuperUser, NoSuperUser, CreateDB, NoCreateDB, + CreateRole, + NoCreateRole, CreateUser, NoCreateUser, + Inherit, + NoInherit, Login, NoLogin, Admin, @@ -1046,8 +1113,12 @@ impl fmt::Display for UserOption { UserOption::NoSuperUser => write!(f, "NOSUPERUSER"), UserOption::CreateDB => write!(f, "CREATEDB"), UserOption::NoCreateDB => write!(f, "NOCREATEDB"), + UserOption::CreateRole => write!(f, "CREATEROLE"), + UserOption::NoCreateRole => write!(f, "NOCREATEROLE"), UserOption::CreateUser => write!(f, "CREATEUSER"), UserOption::NoCreateUser => write!(f, "NOCREATEUSER"), + UserOption::Inherit => write!(f, "INHERIT"), + UserOption::NoInherit => write!(f, "NOINHERIT"), UserOption::Login => write!(f, "LOGIN"), UserOption::NoLogin => write!(f, "NOLOGIN"), UserOption::Admin => write!(f, "ADMIN"), @@ -1069,7 +1140,9 @@ pub struct UserOptions(pub Vec); struct UserOptionsBuilder { super_user: Option, create_db: Option, + create_role: Option, create_user: Option, + inherit: Option, login: Option, admin: Option, password: Option, @@ -1084,9 +1157,15 @@ impl UserOptionsBuilder { if let Some(option) = self.create_db { options.push(option); } + if let Some(option) = self.create_role { + options.push(option); + } if let Some(option) = self.create_user { options.push(option); } + if let Some(option) = self.inherit { + options.push(option); + } if let Some(option) = self.login { options.push(option); } @@ -1124,8 +1203,12 @@ impl ParseTo for UserOptions { "nosuperuser" => (&mut builder.super_user, UserOption::NoSuperUser), "createdb" => (&mut builder.create_db, UserOption::CreateDB), "nocreatedb" => (&mut builder.create_db, UserOption::NoCreateDB), + "createrole" => (&mut builder.create_role, UserOption::CreateRole), + "nocreaterole" => (&mut builder.create_role, UserOption::NoCreateRole), "createuser" => (&mut builder.create_user, UserOption::CreateUser), "nocreateuser" => (&mut builder.create_user, UserOption::NoCreateUser), + "inherit" => (&mut builder.inherit, UserOption::Inherit), + "noinherit" => (&mut builder.inherit, UserOption::NoInherit), "login" => (&mut builder.login, UserOption::Login), "nologin" => (&mut builder.login, UserOption::NoLogin), "admin" => (&mut builder.admin, UserOption::Admin), diff --git a/src/sqlparser/src/keywords.rs b/src/sqlparser/src/keywords.rs index fdb216d94913d..f5880daadb577 100644 --- a/src/sqlparser/src/keywords.rs +++ b/src/sqlparser/src/keywords.rs @@ -69,6 +69,7 @@ define_keywords!( ACTION, ADAPTIVE, ADD, + ADMIN, AGGREGATE, ALL, ALLOCATE, @@ -434,6 +435,7 @@ define_keywords!( RETURNS, REVOKE, RIGHT, + ROLE, ROLLBACK, ROLLUP, ROW, diff --git a/src/sqlparser/src/parser.rs b/src/sqlparser/src/parser.rs index b1fc32709cfaf..3a9907db7ab38 100644 --- a/src/sqlparser/src/parser.rs +++ b/src/sqlparser/src/parser.rs @@ -317,6 +317,7 @@ impl Parser<'_> { Keyword::ALTER => Ok(self.parse_alter()?), Keyword::COPY => Ok(self.parse_copy()?), Keyword::SET => Ok(self.parse_set()?), + Keyword::RESET => Ok(self.parse_reset()?), Keyword::SHOW => { if self.parse_keyword(Keyword::CREATE) { Ok(self.parse_show_create()?) @@ -1990,6 +1991,8 @@ impl Parser<'_> { self.parse_create_database() } else if self.parse_keyword(Keyword::USER) { self.parse_create_user() + } else if self.parse_keyword(Keyword::ROLE) { + self.parse_create_role() } else if self.parse_keyword(Keyword::SECRET) { self.parse_create_secret() } else { @@ -2397,6 +2400,10 @@ impl Parser<'_> { Ok(Statement::CreateUser(CreateUserStatement::parse_to(self)?)) } + fn parse_create_role(&mut self) -> ModalResult { + Ok(Statement::CreateRole(CreateUserStatement::parse_to(self)?)) + } + fn parse_create_secret(&mut self) -> ModalResult { Ok(Statement::CreateSecret { stmt: CreateSecretStatement::parse_to(self)?, @@ -3145,6 +3152,8 @@ impl Parser<'_> { self.parse_alter_connection() } else if self.parse_keyword(Keyword::USER) { self.parse_alter_user() + } else if self.parse_keyword(Keyword::ROLE) { + self.parse_alter_role() } else if self.parse_keyword(Keyword::SYSTEM) { self.parse_alter_system() } else if self.parse_keyword(Keyword::SUBSCRIPTION) { @@ -3217,6 +3226,10 @@ impl Parser<'_> { Ok(Statement::AlterUser(AlterUserStatement::parse_to(self)?)) } + pub fn parse_alter_role(&mut self) -> ModalResult { + Ok(Statement::AlterRole(AlterUserStatement::parse_to(self)?)) + } + pub fn parse_alter_table(&mut self) -> ModalResult { let _ = self.parse_keyword(Keyword::ONLY); let table_name = self.parse_object_name()?; @@ -5001,7 +5014,23 @@ impl Parser<'_> { pub fn parse_set(&mut self) -> ModalResult { let modifier = self.parse_one_of_keywords(&[Keyword::SESSION, Keyword::LOCAL]); - if self.parse_keywords(&[Keyword::TIME, Keyword::ZONE]) { + if self.parse_keyword(Keyword::ROLE) { + let context_modifier = match modifier { + Some(Keyword::SESSION) => Some(RoleContextModifier::Session), + Some(Keyword::LOCAL) => Some(RoleContextModifier::Local), + _ => None, + }; + let role_name = if self.parse_keyword(Keyword::NONE) { + SetRoleSpec::None + } else { + SetRoleSpec::Name(self.parse_identifier()?) + }; + + Ok(Statement::SetRole { + context_modifier, + role_name, + }) + } else if self.parse_keywords(&[Keyword::TIME, Keyword::ZONE]) { let value = alt(( Keyword::DEFAULT.value(SetTimeZoneValue::Default), Keyword::LOCAL.value(SetTimeZoneValue::Local), @@ -5067,6 +5096,14 @@ impl Parser<'_> { } } + pub fn parse_reset(&mut self) -> ModalResult { + if self.parse_keyword(Keyword::ROLE) { + Ok(Statement::ResetRole) + } else { + self.expected("ROLE after RESET") + } + } + /// If have `databases`,`tables`,`columns`,`schemas` and `materialized views` after show, /// return `Statement::ShowCommand` or `Statement::ShowColumn`, /// otherwise, return `Statement::ShowVariable`. @@ -5555,6 +5592,28 @@ impl Parser<'_> { /// Parse a GRANT statement. pub fn parse_grant(&mut self) -> ModalResult { + if !self.grant_revoke_looks_like_object_privilege(Keyword::TO) { + let roles = self.parse_comma_separated(Parser::parse_identifier)?; + self.expect_keyword(Keyword::TO)?; + let grantees = self.parse_comma_separated(Parser::parse_identifier)?; + let role_options = if self.parse_keyword(Keyword::WITH) { + self.parse_comma_separated(Parser::parse_role_option_spec)? + } else { + vec![] + }; + let granted_by = if self.parse_keywords(&[Keyword::GRANTED, Keyword::BY]) { + Some(self.parse_identifier()?) + } else { + None + }; + return Ok(Statement::GrantRole { + roles, + grantees, + role_options, + granted_by, + }); + } + let (privileges, objects) = self.parse_grant_revoke_privileges_objects()?; self.expect_keyword(Keyword::TO)?; @@ -5563,9 +5622,11 @@ impl Parser<'_> { let with_grant_option = self.parse_keywords(&[Keyword::WITH, Keyword::GRANT, Keyword::OPTION]); - let granted_by = self - .parse_keywords(&[Keyword::GRANTED, Keyword::BY]) - .then(|| self.parse_identifier().unwrap()); + let granted_by = if self.parse_keywords(&[Keyword::GRANTED, Keyword::BY]) { + Some(self.parse_identifier()?) + } else { + None + }; Ok(Statement::Grant { privileges, @@ -5576,6 +5637,30 @@ impl Parser<'_> { }) } + fn grant_revoke_looks_like_object_privilege(&self, role_separator: Keyword) -> bool { + let mut paren_depth = 0usize; + let mut index = 0usize; + + loop { + match self.peek_nth_token(index).token { + Token::EOF | Token::SemiColon => return false, + Token::LParen => paren_depth += 1, + Token::RParen => paren_depth = paren_depth.saturating_sub(1), + Token::Word(word) if paren_depth == 0 => { + if word.keyword == Keyword::ON { + return true; + } + if word.keyword == role_separator { + return false; + } + } + _ => {} + } + + index += 1; + } + } + fn parse_privileges(&mut self) -> ModalResult { let privileges = if self.parse_keyword(Keyword::ALL) { Privileges::All { @@ -5768,14 +5853,54 @@ impl Parser<'_> { pub fn parse_revoke(&mut self) -> ModalResult { let revoke_grant_option = self.parse_keywords(&[Keyword::GRANT, Keyword::OPTION, Keyword::FOR]); + let revoke_role_option = if !revoke_grant_option { + self.parse_role_option_for_clause()? + } else { + None + }; + + let starts_with_privilege = self.grant_revoke_looks_like_object_privilege(Keyword::FROM); + if revoke_grant_option && !starts_with_privilege { + parser_err!( + "GRANT OPTION FOR is not valid for role membership revokes; use ADMIN OPTION FOR" + ); + } + + if revoke_role_option.is_some() || !starts_with_privilege { + let roles = self.parse_comma_separated(Parser::parse_identifier)?; + self.expect_keyword(Keyword::FROM)?; + let grantees = self.parse_comma_separated(Parser::parse_identifier)?; + let granted_by = if self.parse_keywords(&[Keyword::GRANTED, Keyword::BY]) { + Some(self.parse_identifier()?) + } else { + None + }; + + let cascade = self.parse_keyword(Keyword::CASCADE); + let restrict = self.parse_keyword(Keyword::RESTRICT); + if cascade && restrict { + parser_err!("Cannot specify both CASCADE and RESTRICT in REVOKE"); + } + + return Ok(Statement::RevokeRole { + roles, + grantees, + revoke_role_option, + granted_by, + cascade, + }); + } + let (privileges, objects) = self.parse_grant_revoke_privileges_objects()?; self.expect_keyword(Keyword::FROM)?; let grantees = self.parse_comma_separated(Parser::parse_identifier)?; - let granted_by = self - .parse_keywords(&[Keyword::GRANTED, Keyword::BY]) - .then(|| self.parse_identifier().unwrap()); + let granted_by = if self.parse_keywords(&[Keyword::GRANTED, Keyword::BY]) { + Some(self.parse_identifier()?) + } else { + None + }; let cascade = self.parse_keyword(Keyword::CASCADE); let restrict = self.parse_keyword(Keyword::RESTRICT); @@ -5793,6 +5918,45 @@ impl Parser<'_> { }) } + fn parse_role_option_for_clause(&mut self) -> ModalResult> { + let kind = match &self.peek_token().token { + Token::Word(word) => match word.value.to_ascii_lowercase().as_str() { + "admin" => Some(RoleOptionKind::Admin), + "inherit" => Some(RoleOptionKind::Inherit), + "set" => Some(RoleOptionKind::Set), + _ => None, + }, + _ => None, + }; + let Some(kind) = kind else { + return Ok(None); + }; + if !self.peek_nth_any_of_keywords(1, &[Keyword::OPTION]) { + return Ok(None); + } + self.next_token(); + self.expect_keyword(Keyword::OPTION)?; + self.expect_keyword(Keyword::FOR)?; + Ok(Some(kind)) + } + + fn parse_role_option_spec(&mut self) -> ModalResult { + let option = self.parse_identifier()?; + let kind = match option.real_value().as_str() { + "admin" => RoleOptionKind::Admin, + "inherit" => RoleOptionKind::Inherit, + "set" => RoleOptionKind::Set, + _ => parser_err!("expected ADMIN, INHERIT or SET, found {}", option), + }; + let value = if matches!(kind, RoleOptionKind::Admin) && self.parse_keyword(Keyword::OPTION) + { + true + } else { + self.parse_boolean()? + }; + Ok(RoleOptionSpec { kind, value }) + } + fn parse_privilege_object_types(&mut self) -> ModalResult { let object_type = if self.parse_keyword(Keyword::TABLES) { PrivilegeObjectType::Tables diff --git a/src/sqlparser/tests/sqlparser_common.rs b/src/sqlparser/tests/sqlparser_common.rs index 16bbd83225637..cf4174598dac7 100644 --- a/src/sqlparser/tests/sqlparser_common.rs +++ b/src/sqlparser/tests/sqlparser_common.rs @@ -3508,6 +3508,32 @@ fn parse_drop_view() { } } +#[test] +fn parse_drop_role() { + let sql = "DROP ROLE analytics"; + match verified_stmt(sql) { + Statement::Drop(stmt) => { + assert_eq!( + ObjectName(vec![Ident::new_unchecked("analytics")]), + stmt.object_name + ); + assert_eq!(ObjectType::Role, stmt.object_type); + assert!(!stmt.if_exists); + assert_eq!(stmt.drop_mode, AstOption::None); + } + _ => unreachable!(), + } + + let sql = "DROP ROLE IF EXISTS analytics"; + match verified_stmt(sql) { + Statement::Drop(stmt) => { + assert_eq!(ObjectType::Role, stmt.object_type); + assert!(stmt.if_exists); + } + _ => unreachable!(), + } +} + #[test] fn parse_materialized_drop_view() { let sql = "DROP MATERIALIZED VIEW mymview"; @@ -3548,6 +3574,47 @@ fn parse_create_user() { } } +#[test] +fn parse_create_role() { + let sql = "CREATE ROLE analytics WITH CREATEDB CREATEROLE INHERIT NOLOGIN"; + match verified_stmt(sql) { + Statement::CreateRole(stmt) => { + assert_eq!( + ObjectName(vec![Ident::new_unchecked("analytics")]), + stmt.user_name + ); + assert_eq!( + stmt.with_options.0, + vec![ + UserOption::CreateDB, + UserOption::CreateRole, + UserOption::Inherit, + UserOption::NoLogin, + ] + ); + } + _ => unreachable!(), + } +} + +#[test] +fn parse_alter_role() { + let sql = "ALTER ROLE analytics RENAME TO reporting"; + match verified_stmt(sql) { + Statement::AlterRole(stmt) => { + assert_eq!( + ObjectName(vec![Ident::new_unchecked("analytics")]), + stmt.user_name + ); + assert_eq!( + stmt.mode, + AlterUserMode::Rename(ObjectName(vec![Ident::new_unchecked("reporting")])) + ); + } + _ => unreachable!(), + } +} + #[test] fn parse_invalid_subquery_without_parens() { let res = parse_sql_statements("SELECT SELECT 1 FROM bar WHERE 1=1 FROM baz"); @@ -4077,6 +4144,43 @@ fn parse_grant() { } } +#[test] +fn parse_grant_role() { + let sql = "GRANT role_a, role_b TO user_a, user_b WITH ADMIN OPTION"; + match verified_stmt(sql) { + Statement::GrantRole { + roles, + grantees, + role_options, + granted_by, + } => { + assert_eq!( + roles, + vec![ + Ident::new_unchecked("role_a"), + Ident::new_unchecked("role_b") + ] + ); + assert_eq!( + grantees, + vec![ + Ident::new_unchecked("user_a"), + Ident::new_unchecked("user_b") + ] + ); + assert_eq!( + role_options, + vec![RoleOptionSpec { + kind: RoleOptionKind::Admin, + value: true + }] + ); + assert_eq!(granted_by, None); + } + _ => unreachable!(), + } +} + #[test] fn test_revoke() { let sql = "REVOKE ALL PRIVILEGES ON users, auth FROM analyst CASCADE"; @@ -4111,6 +4215,233 @@ fn test_revoke() { } } +#[test] +fn parse_revoke_role() { + let sql = "REVOKE ADMIN OPTION FOR role_a FROM user_a CASCADE"; + match verified_stmt(sql) { + Statement::RevokeRole { + roles, + grantees, + revoke_role_option, + granted_by, + cascade, + } => { + assert_eq!(roles, vec![Ident::new_unchecked("role_a")]); + assert_eq!(grantees, vec![Ident::new_unchecked("user_a")]); + assert_eq!(revoke_role_option, Some(RoleOptionKind::Admin)); + assert_eq!(granted_by, None); + assert!(cascade); + } + _ => unreachable!(), + } +} + +#[test] +fn parse_grant_role_membership_options() { + let sql = "GRANT role_a TO user_a WITH INHERIT FALSE, SET FALSE"; + match verified_stmt(sql) { + Statement::GrantRole { + roles, + grantees, + role_options, + granted_by, + } => { + assert_eq!(roles, vec![Ident::new_unchecked("role_a")]); + assert_eq!(grantees, vec![Ident::new_unchecked("user_a")]); + assert_eq!( + role_options, + vec![ + RoleOptionSpec { + kind: RoleOptionKind::Inherit, + value: false + }, + RoleOptionSpec { + kind: RoleOptionKind::Set, + value: false + } + ] + ); + assert_eq!(granted_by, None); + } + _ => unreachable!(), + } +} + +#[test] +fn parse_revoke_set_option_for_role() { + let sql = "REVOKE SET OPTION FOR role_a FROM user_a RESTRICT"; + match verified_stmt(sql) { + Statement::RevokeRole { + roles, + grantees, + revoke_role_option, + granted_by, + cascade, + } => { + assert_eq!(roles, vec![Ident::new_unchecked("role_a")]); + assert_eq!(grantees, vec![Ident::new_unchecked("user_a")]); + assert_eq!(revoke_role_option, Some(RoleOptionKind::Set)); + assert_eq!(granted_by, None); + assert!(!cascade); + } + _ => unreachable!(), + } +} + +#[test] +fn parse_grant_role_granted_by() { + let sql = "GRANT role_a TO user_a GRANTED BY current_user"; + match verified_stmt(sql) { + Statement::GrantRole { granted_by, .. } => { + assert_eq!(granted_by, Some(Ident::new_unchecked("current_user"))); + } + _ => unreachable!(), + } +} + +#[test] +fn parse_grant_role_granted_by_requires_identifier() { + let err = parse_sql_statements("GRANT role_a TO user_a GRANTED BY ;") + .unwrap_err() + .to_string(); + assert!(err.contains("expected identifier, found: ;"), "{err}"); +} + +#[test] +fn parse_revoke_role_granted_by() { + let sql = "REVOKE role_a FROM user_a GRANTED BY current_user RESTRICT"; + match verified_stmt(sql) { + Statement::RevokeRole { granted_by, .. } => { + assert_eq!(granted_by, Some(Ident::new_unchecked("current_user"))); + } + _ => unreachable!(), + } +} + +#[test] +fn parse_revoke_role_granted_by_requires_identifier() { + let err = parse_sql_statements("REVOKE role_a FROM user_a GRANTED BY ;") + .unwrap_err() + .to_string(); + assert!(err.contains("expected identifier, found: ;"), "{err}"); +} + +#[test] +fn parse_grant_privilege_granted_by_requires_identifier() { + let err = parse_sql_statements("GRANT SELECT ON abc TO user_a GRANTED BY ;") + .unwrap_err() + .to_string(); + assert!(err.contains("expected identifier, found: ;"), "{err}"); +} + +#[test] +fn parse_revoke_privilege_granted_by_requires_identifier() { + let err = parse_sql_statements("REVOKE SELECT ON abc FROM user_a GRANTED BY ;") + .unwrap_err() + .to_string(); + assert!(err.contains("expected identifier, found: ;"), "{err}"); +} + +#[test] +fn parse_revoke_grant_option_for_role_is_rejected() { + let err = parse_sql_statements("REVOKE GRANT OPTION FOR role_a FROM user_a") + .unwrap_err() + .to_string(); + assert!( + err.contains("GRANT OPTION FOR is not valid for role membership revokes"), + "{err}" + ); +} + +#[test] +fn parse_revoke_role_named_option_words_without_option_clause() { + for role in ["admin", "inherit", "set"] { + let sql = format!("REVOKE {role} FROM user_a RESTRICT"); + match verified_stmt(&sql) { + Statement::RevokeRole { + roles, + grantees, + revoke_role_option, + granted_by, + cascade, + } => { + assert_eq!(roles, vec![Ident::new_unchecked(role)]); + assert_eq!(grantees, vec![Ident::new_unchecked("user_a")]); + assert_eq!(revoke_role_option, None); + assert_eq!(granted_by, None); + assert!(!cascade); + } + _ => unreachable!(), + } + } +} + +#[test] +fn parse_grant_revoke_role_named_privilege_words() { + for role in ["connect", "create", "select", "usage"] { + let grant_sql = format!("GRANT {role} TO user_a"); + match verified_stmt(&grant_sql) { + Statement::GrantRole { + roles, + grantees, + role_options, + granted_by, + } => { + assert_eq!(roles, vec![Ident::new_unchecked(role)]); + assert_eq!(grantees, vec![Ident::new_unchecked("user_a")]); + assert!(role_options.is_empty()); + assert_eq!(granted_by, None); + } + _ => unreachable!(), + } + + let revoke_sql = format!("REVOKE {role} FROM user_a RESTRICT"); + match verified_stmt(&revoke_sql) { + Statement::RevokeRole { + roles, + grantees, + revoke_role_option, + granted_by, + cascade, + } => { + assert_eq!(roles, vec![Ident::new_unchecked(role)]); + assert_eq!(grantees, vec![Ident::new_unchecked("user_a")]); + assert_eq!(revoke_role_option, None); + assert_eq!(granted_by, None); + assert!(!cascade); + } + _ => unreachable!(), + } + } +} + +#[test] +fn parse_set_role() { + let sql = "SET LOCAL ROLE analytics"; + match verified_stmt(sql) { + Statement::SetRole { + context_modifier, + role_name, + } => { + assert_eq!(context_modifier, Some(RoleContextModifier::Local)); + assert_eq!( + role_name, + SetRoleSpec::Name(Ident::new_unchecked("analytics")) + ); + } + _ => unreachable!(), + } +} + +#[test] +fn parse_reset_role() { + let sql = "RESET ROLE"; + match verified_stmt(sql) { + Statement::ResetRole => {} + _ => unreachable!(), + } +} + #[test] fn all_keywords_sorted() { // assert!(ALL_KEYWORDS.is_sorted()) diff --git a/src/sqlparser/tests/testdata/drop.yaml b/src/sqlparser/tests/testdata/drop.yaml index 565e1bf78d85a..8b5df67c942dd 100644 --- a/src/sqlparser/tests/testdata/drop.yaml +++ b/src/sqlparser/tests/testdata/drop.yaml @@ -16,6 +16,10 @@ formatted_sql: DROP USER user - input: DROP USER IF EXISTS user formatted_sql: DROP USER IF EXISTS user +- input: DROP ROLE role_a + formatted_sql: DROP ROLE role_a +- input: DROP ROLE IF EXISTS role_a + formatted_sql: DROP ROLE IF EXISTS role_a - input: DROP SECRET secret formatted_sql: DROP SECRET secret - input: DROP SECRET IF EXISTS secret diff --git a/src/sqlparser/tests/testdata/role.yaml b/src/sqlparser/tests/testdata/role.yaml new file mode 100644 index 0000000000000..0bf149820899c --- /dev/null +++ b/src/sqlparser/tests/testdata/role.yaml @@ -0,0 +1,55 @@ +# This file is automatically generated by `src/sqlparser/tests/parser_test.rs`. +- input: CREATE ROLE analyst + formatted_sql: CREATE ROLE analyst +- input: CREATE ROLE admin_role WITH SUPERUSER + formatted_sql: CREATE ROLE admin_role WITH SUPERUSER +- input: CREATE ROLE db_owner WITH CREATEDB + formatted_sql: CREATE ROLE db_owner WITH CREATEDB +- input: CREATE ROLE role_admin WITH CREATEROLE + formatted_sql: CREATE ROLE role_admin WITH CREATEROLE +- input: CREATE ROLE inherited_role WITH INHERIT + formatted_sql: CREATE ROLE inherited_role WITH INHERIT +- input: CREATE ROLE login_role WITH LOGIN PASSWORD 'secret' + formatted_sql: CREATE ROLE login_role WITH LOGIN PASSWORD 'secret' +- input: CREATE ROLE nologin_role WITH NOLOGIN + formatted_sql: CREATE ROLE nologin_role WITH NOLOGIN +- input: ALTER ROLE admin_role WITH NOSUPERUSER + formatted_sql: ALTER ROLE admin_role WITH NOSUPERUSER +- input: ALTER ROLE db_owner WITH NOCREATEDB + formatted_sql: ALTER ROLE db_owner WITH NOCREATEDB +- input: ALTER ROLE role_admin WITH NOCREATEROLE + formatted_sql: ALTER ROLE role_admin WITH NOCREATEROLE +- input: ALTER ROLE inherited_role WITH NOINHERIT + formatted_sql: ALTER ROLE inherited_role WITH NOINHERIT +- input: ALTER ROLE login_role WITH NOLOGIN + formatted_sql: ALTER ROLE login_role WITH NOLOGIN +- input: GRANT analyst TO login_role + formatted_sql: GRANT analyst TO login_role +- input: GRANT analyst TO login_role WITH ADMIN OPTION + formatted_sql: GRANT analyst TO login_role WITH ADMIN OPTION +- input: GRANT analyst TO login_role WITH INHERIT FALSE, SET FALSE + formatted_sql: GRANT analyst TO login_role WITH INHERIT FALSE, SET FALSE +- input: GRANT analyst TO login_role GRANTED BY current_user + formatted_sql: GRANT analyst TO login_role GRANTED BY current_user +- input: REVOKE analyst FROM login_role + formatted_sql: REVOKE analyst FROM login_role RESTRICT +- input: REVOKE SET OPTION FOR analyst FROM login_role + formatted_sql: REVOKE SET OPTION FOR analyst FROM login_role RESTRICT +- input: REVOKE analyst FROM login_role GRANTED BY current_user + formatted_sql: REVOKE analyst FROM login_role GRANTED BY current_user RESTRICT +- input: SET ROLE analyst + formatted_sql: SET ROLE analyst +- input: SET ROLE NONE + formatted_sql: SET ROLE NONE +- input: RESET ROLE + formatted_sql: RESET ROLE +- input: CREATE ROLE conflicting_role WITH LOGIN NOLOGIN + error_msg: |- + sql parser error: conflicting or redundant options + LINE 1: CREATE ROLE conflicting_role WITH LOGIN NOLOGIN + ^ +- input: CREATE ROLE conflicting_role WITH INHERIT NOINHERIT + error_msg: |- + sql parser error: conflicting or redundant options + LINE 1: CREATE ROLE conflicting_role WITH INHERIT NOINHERIT + ^ diff --git a/src/utils/pgwire/src/pg_response.rs b/src/utils/pgwire/src/pg_response.rs index 3054c9ed76963..de319c80c3adb 100644 --- a/src/utils/pgwire/src/pg_response.rs +++ b/src/utils/pgwire/src/pg_response.rs @@ -283,7 +283,9 @@ impl StatementType { Statement::CreateSink { .. } => Ok(StatementType::CREATE_SINK), Statement::CreateFunction { .. } => Ok(StatementType::CREATE_FUNCTION), Statement::CreateDatabase { .. } => Ok(StatementType::CREATE_DATABASE), - Statement::CreateUser { .. } => Ok(StatementType::CREATE_USER), + Statement::CreateUser { .. } | Statement::CreateRole { .. } => { + Ok(StatementType::CREATE_USER) + } Statement::CreateView { materialized, .. } => { if *materialized { Ok(StatementType::CREATE_MATERIALIZED_VIEW) @@ -296,15 +298,24 @@ impl StatementType { Statement::AlterFragment { .. } => Ok(StatementType::ALTER_FRAGMENT), Statement::DropFunction { .. } => Ok(StatementType::DROP_FUNCTION), Statement::Discard(..) => Ok(StatementType::DISCARD), - Statement::SetVariable { .. } => Ok(StatementType::SET_VARIABLE), + Statement::AlterUser { .. } | Statement::AlterRole { .. } => { + Ok(StatementType::UPDATE_USER) + } + Statement::SetVariable { .. } | Statement::SetRole { .. } | Statement::ResetRole => { + Ok(StatementType::SET_VARIABLE) + } Statement::ShowVariable { .. } => Ok(StatementType::SHOW_VARIABLE), Statement::StartTransaction { .. } => Ok(StatementType::START_TRANSACTION), Statement::Begin { .. } => Ok(StatementType::BEGIN), Statement::Abort => Ok(StatementType::ABORT), Statement::Commit { .. } => Ok(StatementType::COMMIT), Statement::Rollback { .. } => Ok(StatementType::ROLLBACK), - Statement::Grant { .. } => Ok(StatementType::GRANT_PRIVILEGE), - Statement::Revoke { .. } => Ok(StatementType::REVOKE_PRIVILEGE), + Statement::Grant { .. } | Statement::GrantRole { .. } => { + Ok(StatementType::GRANT_PRIVILEGE) + } + Statement::Revoke { .. } | Statement::RevokeRole { .. } => { + Ok(StatementType::REVOKE_PRIVILEGE) + } Statement::Describe { .. } => Ok(StatementType::DESCRIBE), Statement::ShowCreateObject { .. } | Statement::ShowObjects { .. } => { Ok(StatementType::SHOW_COMMAND) @@ -320,6 +331,7 @@ impl StatementType { risingwave_sqlparser::ast::ObjectType::Source => Ok(StatementType::DROP_SOURCE), risingwave_sqlparser::ast::ObjectType::Sink => Ok(StatementType::DROP_SINK), risingwave_sqlparser::ast::ObjectType::Database => Ok(StatementType::DROP_DATABASE), + risingwave_sqlparser::ast::ObjectType::Role => Ok(StatementType::DROP_USER), risingwave_sqlparser::ast::ObjectType::User => Ok(StatementType::DROP_USER), risingwave_sqlparser::ast::ObjectType::Connection => { Ok(StatementType::DROP_CONNECTION)