TPT-4175: cli: Update interactive config token access check - #881
Conversation
mgwoj
added
the
improvement
There was a problem hiding this comment.
Pull request overview
This PR updates the CLI’s token “full access” check used during interactive configuration to avoid failing hard when /profile/grants is inaccessible (notably for IAM-enrolled users).
Changes:
- Update
_check_full_accessto treat HTTP 403 from/profile/grantsas a non-fatal response and interpret it as “not full access”. - Add unit tests covering
_check_full_accessbehavior for 204/200/403 responses.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
linodecli/configuration/auth.py |
Adjusts response handling for /profile/grants to allow 403 without exiting. |
tests/unit/test_configuration.py |
Adds unit tests verifying _check_full_access return values for key status codes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # IAM-enrolled users receive a 403 from /profile/grants since that | ||
| # endpoint is not accessible to them. Treat 403 as a valid response | ||
| # (i.e. not full access) rather than a fatal error. | ||
| _handle_response_status( | ||
| result, | ||
| exit_on_error=True, | ||
| status_validator=lambda status: status == 403, | ||
| ) |
There was a problem hiding this comment.
Treating a 403 from /profile/grants as a non-fatal response makes _check_full_access return False for IAM-enrolled users. In CLIConfig.configure() this value gates the /account/users lookup, so IAM users will never be offered authorized_users selection even if they otherwise have the needed account permissions. If the intent is to still support configuring authorized users for IAM tokens, consider falling back to a different capability check (e.g. attempt /account/users with a status_validator for 401/403) or decouple the authorized-users prompt from _check_full_access when the grants endpoint is inaccessible.
There was a problem hiding this comment.
@jriddle-linode Thoughts on this? I can't seem to reproduce the issue that requires this change but I might not be properly enrolled in IAM
There was a problem hiding this comment.
The best solution is probably just checking https://techdocs.akamai.com/linode-api/reference/get-ssh-keys and if we get an error leave out the option.
There was a problem hiding this comment.
ok, I have followed your suggestion
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…g-token-access-check
…g-token-access-check
jriddle-linode
left a comment
jriddle-linode
left a comment
There was a problem hiding this comment.
LGTM! Works locally thank you!
…g-token-access-check
There was a problem hiding this comment.
🟡 Changes recommended
Unexpected validator failures are currently treated as successful access checks and must be handled normally.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
Update SSH keys status handling to check for specific error codes. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Add coverage for /profile/sshkeys returning 200 and /account/users returning 403.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
linodecli/configuration/config.py:457
- The new
403allowance on/account/usersis not exercised by the added tests: the only 403 case returns from the/profile/sshkeyscapability check before this request is made. Please add a case with/profile/sshkeysreturning 200 and/account/usersreturning 403 to verify configuration continues and omitsauthorized_usersfor this permission combination.
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
Accepted 401/403 responses with empty or non-JSON bodies can still fail before the status handling; the request helper and tests need updating.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
linodecli/configuration/config.py:449
- These statuses are treated as expected, but
_do_requeststill unconditionally callsresult.json()after running the validator (auth.py:148-152). A valid 401/403 response with an empty or non-JSON body will raise before this gate can skip the prompt. Make the request helper tolerate bodyless accepted responses and add an empty-body test.
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
…g-token-access-check
📝 Description
AM Users wont have access to /profile/grants so we need to come up with a new way to check the access level of an account so it can add auth users.
✔️ How to Test