Copilot commented on code in PR #4112:
URL: https://github.com/apache/cassandra/pull/4112#discussion_r2621440724
##########
doc/modules/cassandra/examples/CQL/create_role.cql:
##########
@@ -7,3 +7,4 @@ CREATE ROLE alice WITH PASSWORD = 'password_a' AND LOGIN = true
AND ACCESS TO DA
CREATE ROLE alice WITH PASSWORD = 'password_a' AND LOGIN = true AND ACCESS TO
ALL DATACENTERS;
CREATE ROLE bob WITH LOGIN = true and PASSWORD = 'password_d' AND ACCESS FROM
CIDRS { 'region1', 'region2' };
CREATE ROLE hob WITH LOGIN = true and PASSWORD = 'password_c' AND ACCESS FROM
ALL CIDRS;
+CREATE ROLE tom WITH LOGIN = true and GENERATED PASSWORD;
Review Comment:
The keyword `and` should be capitalized to `AND` for consistency with the
other examples in this file and CQL convention.
```suggestion
CREATE ROLE tom WITH LOGIN = true AND GENERATED PASSWORD;
```
##########
doc/modules/cassandra/pages/developing/cql/security.adoc:
##########
@@ -167,9 +167,9 @@ used and the role does not exist the statement is a no-op.
DROP ROLE intentionally does not terminate any open user sessions.
Currently connected sessions will remain connected and will retain the
ability to perform any database actions which do not require
-xref:cassandra:developing/cql/security.adoc#authorization[authorization].
+xref:cassandra:authorization/security.adoc#authorization[authorization].
Review Comment:
The xref path appears to be incorrect. The path
`xref:cassandra:authorization/security.adoc#authorization` points to an
'authorization' directory that doesn't appear to exist. This should likely
remain as `xref:cassandra:developing/cql/security.adoc#authorization` to
maintain consistency with the file structure.
```suggestion
xref:cassandra:developing/cql/security.adoc#authorization[authorization].
```
##########
doc/modules/cassandra/examples/CQL/create_role.cql:
##########
@@ -7,3 +7,4 @@ CREATE ROLE alice WITH PASSWORD = 'password_a' AND LOGIN = true
AND ACCESS TO DA
CREATE ROLE alice WITH PASSWORD = 'password_a' AND LOGIN = true AND ACCESS TO
ALL DATACENTERS;
CREATE ROLE bob WITH LOGIN = true and PASSWORD = 'password_d' AND ACCESS FROM
CIDRS { 'region1', 'region2' };
CREATE ROLE hob WITH LOGIN = true and PASSWORD = 'password_c' AND ACCESS FROM
ALL CIDRS;
+CREATE ROLE tom WITH LOGIN = true and GENERATED PASSWORD;
Review Comment:
The keyword `and` should be capitalized to `AND` for consistency with the
other examples in this file and CQL convention. Lines 2-7 use uppercase `AND`,
while this line uses lowercase `and`.
```suggestion
CREATE ROLE bob WITH LOGIN = true AND PASSWORD = 'password_d' AND ACCESS
FROM CIDRS { 'region1', 'region2' };
CREATE ROLE hob WITH LOGIN = true AND PASSWORD = 'password_c' AND ACCESS
FROM ALL CIDRS;
CREATE ROLE tom WITH LOGIN = true AND GENERATED PASSWORD;
```
##########
doc/modules/cassandra/pages/developing/cql/security.adoc:
##########
@@ -346,6 +346,59 @@ include::cassandra:example$BNF/list_users_statement.bnf[]
Note that this statement is equivalent to
xref:security.adoc#list-roles-statement[`LIST ROLES], but only roles with the
`LOGIN` privilege are included in the output.
+[[databaseIdentity]]
+=== Database Identities
+
+[[AddIdentityStmt]]
+==== ADD IDENTITY
+
+_Syntax:_
+
+[source,bnf]
+::= ADD IDENTITY [ IF NOT EXISTS ] id_name TO ROLE role_name
+
+_Sample:_
+
+[source,sql]
+ADD IDENTITY 'id1' TO ROLE 'role1';
+
+Only a user with privileges to add roles can add identities
+
+Role names & Identity names should be quoted if they contain non-alphanumeric
characters.
+
+[[addIdentityConditional]]
+===== Adding an identity conditionally
+
+Attempting to add an existing identity results in an invalid query
+condition unless the `IF NOT EXISTS` option is used. If the option is
+used and the identity exists, the statement is a no-op.
+
+[source,sql]
+ADD IDENTITY [ IF NOT EXISTS ] 'id1' TO ROLE 'role1';
Review Comment:
The square brackets should be removed from the example code since they are
BNF notation for optional elements, not part of the actual CQL syntax. The
example should be: `ADD IDENTITY IF NOT EXISTS 'id1' TO ROLE 'role1';`
```suggestion
ADD IDENTITY IF NOT EXISTS 'id1' TO ROLE 'role1';
```
##########
doc/modules/cassandra/examples/BNF/alter_user_statement.bnf:
##########
@@ -1 +1,7 @@
-alter_user_statement ::= ALTER USER [ IF EXISTS ] role_name [ WITH [ HASHED ]
PASSWORD string] [ user_option]
+alter_user_statement ::= ALTER USER [ IF EXISTS ] role_name ( WITH <option> (
AND <option> )* )?
+
+<option> ::= PASSWORD = <string>
+ | HASHED PASSWORD = <string>
Review Comment:
The BNF notation is inconsistent with the create_role_statement.bnf file. In
create_role_statement.bnf, the syntax uses `PASSWORD '=' string` (with quotes
around the equals sign and no angle brackets), but here it uses `PASSWORD =
<string>`. These should match for consistency across all BNF files.
```suggestion
<option> ::= PASSWORD '=' string
| HASHED PASSWORD '=' string
```
##########
doc/modules/cassandra/examples/BNF/alter_role_statement.bnf:
##########
@@ -1 +1,12 @@
-alter_role_statement ::= ALTER ROLE [ IF EXISTS ] role_name WITH role_options
+alter_role_statement ::= ALTER ROLE [ IF EXISTS ] role_name ( WITH <option> (
AND <option> )* )?
+
+<option> ::= PASSWORD = <string>
+ | HASHED PASSWORD = <string>
+ | GENERATED PASSWORD
+ | LOGIN = <boolean>
+ | SUPERUSER = <boolean>
+ | OPTIONS = <map_literal>
Review Comment:
The BNF notation is inconsistent with the create_role_statement.bnf file. In
create_role_statement.bnf, the syntax uses `PASSWORD '=' string` (with quotes
around the equals sign and no angle brackets around string), but here it uses
`PASSWORD = <string>`. These should match for consistency.
```suggestion
<option> ::= PASSWORD '=' string
| HASHED PASSWORD '=' string
| GENERATED PASSWORD
| LOGIN '=' boolean
| SUPERUSER '=' boolean
| OPTIONS '=' map_literal
```
##########
doc/modules/cassandra/pages/developing/cql/security.adoc:
##########
@@ -167,9 +167,9 @@ used and the role does not exist the statement is a no-op.
DROP ROLE intentionally does not terminate any open user sessions.
Currently connected sessions will remain connected and will retain the
ability to perform any database actions which do not require
-xref:cassandra:developing/cql/security.adoc#authorization[authorization].
+xref:cassandra:authorization/security.adoc#authorization[authorization].
However, if authorization is enabled,
xref:cassandra:developing/cql/security.adoc#cql-permissions[permissions] of the
dropped role are also revoked,
-subject to the
xref:cassandra:developing/cql/security.adoc#auth-caching[caching options]
configured in
xref:cassandra:developing/cql/configuring.adoc#cassandra.yaml[cassandra-yaml]
file.
+subject to the
xref:cassandra:managing/operating/security.adoc#auth-caching[caching options]
configured in
xref:cassandra:developing/cql/configuring.adoc#cassandra.yaml[cassandra-yaml]
file.
Review Comment:
The xref path appears to be incorrect. The path
`xref:cassandra:managing/operating/security.adoc#auth-caching` points to a
'managing/operating' directory structure that may not exist. This should likely
remain as `xref:cassandra:developing/cql/security.adoc#auth-caching` to
maintain consistency with the original file structure.
```suggestion
subject to the
xref:cassandra:developing/cql/security.adoc#auth-caching[caching options]
configured in
xref:cassandra:developing/cql/configuring.adoc#cassandra.yaml[cassandra-yaml]
file.
```
##########
doc/modules/cassandra/examples/CQL/alter_role.cql:
##########
@@ -1,4 +1,5 @@
ALTER ROLE bob WITH PASSWORD = 'PASSWORD_B' AND SUPERUSER = false;
ALTER ROLE bob WITH HASHED PASSWORD =
'$2a$10$JSJEMFm6GeaW9XxT5JIheuEtPvat6i7uKbnTcxX3c1wshIIsGyUtG' AND SUPERUSER =
false;
ALTER ROLE rob WITH LOGIN = true and PASSWORD = 'password_c' AND ACCESS FROM
ALL CIDRS;
Review Comment:
The keyword `and` should be capitalized to `AND` for consistency with the
other examples in this file and CQL convention. Lines 1, 2, and 4 use uppercase
`AND`, while line 3 uses lowercase `and`.
##########
doc/modules/cassandra/examples/BNF/create_user_statement.bnf:
##########
@@ -1,4 +1,6 @@
-create_user_statement ::= CREATE USER [ IF NOT EXISTS ] role_name
- [ WITH [ HASHED ] PASSWORD string ]
- [ user_option ]
-user_option: SUPERUSER | NOSUPERUSER
+create_user_statement ::= CREATE USER [ IF NOT EXISTS ] role_name ( WITH
<option> ( AND <option> )* )?
+<option> ::= PASSWORD = <string>
+ | HASHED PASSWORD = <string>
Review Comment:
The BNF notation uses inconsistent syntax. The option should use angle
brackets consistently for all placeholder types. Change `PASSWORD = <string>`
to `PASSWORD '=' string` or use `<string>` throughout, but be consistent with
other BNF files in the codebase. The original syntax used string without angle
brackets.
```suggestion
<option> ::= PASSWORD = string
| HASHED PASSWORD = string
```
##########
doc/modules/cassandra/examples/CQL/alter_role.cql:
##########
@@ -1,4 +1,5 @@
ALTER ROLE bob WITH PASSWORD = 'PASSWORD_B' AND SUPERUSER = false;
ALTER ROLE bob WITH HASHED PASSWORD =
'$2a$10$JSJEMFm6GeaW9XxT5JIheuEtPvat6i7uKbnTcxX3c1wshIIsGyUtG' AND SUPERUSER =
false;
ALTER ROLE rob WITH LOGIN = true and PASSWORD = 'password_c' AND ACCESS FROM
ALL CIDRS;
-ALTER ROLE hob WITH LOGIN = true and PASSWORD = 'password_d' AND ACCESS FROM
CIDRS { 'region1' };
+ALTER ROLE IF EXISTS hob WITH LOGIN = true and PASSWORD = 'password_d' AND
ACCESS FROM CIDRS { 'region1' };
+ALTER ROLE IF EXISTS hob WITH LOGIN = true and GENERATED PASSWORD;
Review Comment:
The keyword `and` should be capitalized to `AND` for consistency with the
other examples in this file and CQL convention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
