Repository navigation
add sync option - #13
Conversation
village-tom
left a comment
village-tom
left a comment
There was a problem hiding this comment.
Overall looks good, just one doc comment and questions about the python script and keys.
| - `SYNC`: the token's roles become the account's **exact** granted set — grant | ||
| the ones it lacks **and revoke every other role it holds that the token did | ||
| not claim** (a token carrying no roles revokes them all). This makes the | ||
| token issuer the sole source of truth for the account's roles, so a role a | ||
| DBA granted out of band is revoked on the next login too. Use it only where | ||
| that is the intent. **Exception:** an account's **default roles** (set by an | ||
| operator with `ALTER USER … DEFAULT ROLE`) are never revoked — a deliberate | ||
| operator pin outranks the token. (An auto-created account has no default | ||
| role, so this only shields defaults an operator set explicitly.) |
There was a problem hiding this comment.
Let's also mention here that if a role is revoked mid-session it may cause unexpected behavior.
There was a problem hiding this comment.
I can add a test, but this is generic behavior if an admin decides to revoke a role, so nothing specific related to this work per se
There was a problem hiding this comment.
That's fair enough, although I think a DB admin would be more cognizant of revoking a role directly on the database, and somebody updating IdP config might not think it through in the same way. Though if nothing happens to existing sessions then we can leave the documentation as-is.
| @@ -0,0 +1,192 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Why was this added in this PR? Is it supposed to be run manually, or during CI?
There was a problem hiding this comment.
it has nothing to do with this PR, I just discovered I had them and figured I would add them, I can do a separate PR
There was a problem hiding this comment.
Yeah let's do a separate PR if this has nothing to do with the sync option, I'm still unclear on what this script is used for
| @@ -0,0 +1,28 @@ | |||
| -----BEGIN PRIVATE KEY----- | |||
There was a problem hiding this comment.
Why do we need these private keys committed to the repo? They don't present a security risk but they may be flagged in the future. How much work would it be to generate them on the fly instead?
There was a problem hiding this comment.
it has nothing to do with this PR, I just discovered I had them and figured I would add them, I can do a separate PR
3 participants
No description provided.