markhoerth opened a new pull request, #12247:
URL: https://github.com/apache/gravitino/pull/12247

   ### What changes were proposed in this pull request?
   
   A full rewrite of `docs/security/access-control.md`. Every claim on the page 
was checked against the source rather than carried over from the previous 
version. The page goes from 1405 lines to 591.
   
   **Corrections**
   
   - Removed a note in the Overview stating that Gravitino "won't check the 
privileges when Gravitino receives the requests". The server layer carries 117 
`@AuthorizationExpression` annotations, so metadata authorization is enforced. 
The note appears to predate built-in authorization and contradicted the rest of 
the page.
   - Removed a diagram that showed `CREATE_TABLE` bound to a table. 
`CreateTable.canBindTo` uses `SCHEMA_SUPPORTED_TYPES`, so the server rejects 
that binding.
   - Added `gravitino.authorization.impl` and 
`gravitino.authorization.threadPoolSize`, which were missing from the 
configuration table.
   - Corrected `jcasbin.roleCacheSize`, which is applied separately to three 
caches, so real memory use is roughly triple the configured value.
   - Corrected the `MODIFY_TABLE` description, which claimed it grants reads.
   - Fixed `COMSUME_TOPIC`, four unclosed backticks around `USE_MODEL`, a 
garbled row in the conditions table, and two duplicate rows, one of which was a 
mislabelled copy of a different operation.
   - Added the multi-node consequence of `jcasbin.changePollIntervalSecs`: a 
revocation can take up to that interval to take effect on other nodes.
   - Added the behaviour of a failed check, `403 Forbidden`, with `404 Not 
Found` on some read paths so callers cannot probe for objects they cannot see.
   - Added the fact that role object names are not validated, so a typo 
produces a role that grants nothing.
   
   **Removals**
   
   - Five diagrams. Each was stale, wrong, or a rendering of adjacent text. The 
securable-object tree omitted View, Model, and Function while the text listed 
them; the relationship diagram had its arrows reversed and named individuals; 
the role diagram encoded the invalid grant above; the ALLOW/DENY diagram used a 
full screen to repeat one word; the workflow diagram duplicated the walkthrough 
beneath it as pixels.
   - The per-operation REST and Java reference, about 400 lines, which 
duplicated `docs/open-api/` and the published Javadoc and had drifted from 
both. Four of the Java examples did not compile. Replaced with a compact 
endpoint table, three client examples covering the calls whose arguments are 
not obvious from the signature, and links to the generated references.
   - 20 `:::` admonition blocks and 24 tab widgets, per the conventions settled 
on the credential vending page.
   
   **Restructuring**
   
   - 12 top-level sections reduced to 8, nested rather than flat.
   - The 105-row API conditions table, in which 49 rows opened with the same 
sentence, is replaced by two grids plus four short paragraphs for the 
operations that do not follow the pattern.
   - 12 per-object privilege tables consolidated into 2, so columns align.
   - Quick Start reduced to the two properties that enable authorization. The 
full sequence lives in the Walkthrough, which now runs from an empty server to 
a user reading one schema.
   - Authenticator configuration moved to Server Configuration, including the 
point that `principalFields` defaults to `sub` and must produce the names used 
in metalake membership.
   
   ### Why are the changes needed?
   
   The page told readers that Gravitino does not check privileges on incoming 
requests, which is false and appeared under the Overview heading. It documented 
a grant the server rejects, and shipped Java examples that do not compile. 
Anyone reading it to understand the security model was being misinformed, and 
anyone copying from it was being sent into errors.
   
   The page was also unusable in shape: 1405 lines, 12 flat top-level sections, 
and a reference table in which the same sentence was repeated 49 times.
   
   Fix: #(issue)
   
   ### Does this PR introduce _any_ user-facing change?
   
   Documentation only. No API, configuration, or behaviour changes.
   
   Section anchors change. Two inbound links break and are fixed in a follow-up 
PR to keep this one scoped to a single file:
   
   - `docs/webui-v2.md:347` links to `#securable-objects` and `#privilege-types`
   - `docs/security/how-to-use-built-in-idp.md:390` links to `#add-a-user`
   
   ### How was this patch tested?
   
   Documentation only, so no automated tests.
   
   Every factual claim was checked against `main`:
   
   - `api/.../authorization/Privilege.java` and `Privileges.java` for the 
privilege names and the object types each can bind to
   - `core/.../Configs.java` for configuration keys, defaults, and 
since-versions
   - `server/.../web/rest/*.java` and `AuthorizationExpressionConstants.java` 
for the per-operation conditions and for confirming authorization is enforced
   - `core/.../hook/MetalakeHookDispatcher.java` for the creator becoming user 
and owner
   - `core/.../authorization/RoleManager.java` for role creation performing no 
existence check
   - `server-common/.../authentication/SimpleAuthenticator.java` and 
`OAuthConfig.java` for the identity settings
   - `clients/client-java/.../GravitinoClient.java` for the client method 
signatures
   
   Rendering was checked for heading nesting, table widths without horizontal 
scroll, and the four internal anchors resolving.


-- 
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]

Reply via email to