fix: Detect invalid API keys more carefully#792
Conversation
|
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
atheriel
force-pushed
the
aj-more-careful-api-key-validation
branch
from
a13d522 to
a5c8bac
Compare
nealrichardson
left a comment
nealrichardson
left a comment
There was a problem hiding this comment.
I confirmed that this works, I just have some design questions. I'm ok to merge this and make a followup to refactor if there's no other simplification we can make here.
| json_data = result.json_data if isinstance(result.json_data, dict) else {} | ||
| code = json_data.get("code") | ||
| # A service principal or machine identity authenticates successfully but has no | ||
| # associated user, so the v1/user endpoint rejects it with a 403 and error code |
There was a problem hiding this comment.
Is it necessary that we request the v1/user endpoint to validate the API key, or the OIDC token? I noticed that nothing else uses the .me() endpoint other than to check that an API key is valid.
There was a problem hiding this comment.
I also found this confusing. I believe the original point of it was to surface more user-friendly error messages, but I agree that I would personally prefer we just fail later on when attempting an actual operation. That felt like a more invasive behaviour change to make, though.
When credentials come from the command line (or environment variables) rather than the store, we attempt to validate them by hitting the `/v1/user` endpoint, and error out if this fails. It turns out that this doesn't work for machine credentials, which aren't associated with a user. Thankfully the difference between "invalid API key" and "not a user API key" can be detected based on the error code from the response, so this commit draws this exact distinction. Unit tests are included. Signed-off-by: Aaron Jacobs <aaron.jacobs@posit.co>
atheriel
force-pushed
the
aj-more-careful-api-key-validation
branch
from
a5c8bac to
bff5fd5
Compare
2 participants
Intent
When credentials come from the command line (or environment variables) rather than the store, we attempt to validate them by hitting the
/v1/userendpoint, and error out if this fails.It turns out that this doesn't work for machine credentials, which aren't associated with a user.
Thankfully the difference between "invalid API key" and "not a user API key" can be detected based on the error code from the response, so this commit draws this exact distinction.
Type of Change
Automated Tests
Unit tests are included.
Directions for Reviewers
Checklist
rsconnect-python-tests-at-nightworkflow in Connect against this feature branch.