-
Notifications
You must be signed in to change notification settings - Fork 231
cocalc-api: adding tests, some enhancements, etc. #8601
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…re missing return types, generate a better README
…oject lifecycle as part of the cocalc-api tests
…o be admins or add existing users to orgs. Org-admins can create new users inside an org, though.
d87a26b to
d24ec9a
Compare
a89eb5c to
4090399
Compare
… – which is actually something a mere API call via the org management should not do
4090399 to
29dc582
Compare
2f87541 to
52b2394
Compare
…projects. (managing an ephemeral database via the API is a bit tricky)
c0f75c6 to
b5f73cf
Compare
b5f73cf to
95c7274
Compare
b427600 to
6c49b85
Compare
6c49b85 to
5434d8d
Compare
405e4a4 to
7d39dcb
Compare
dd8c5f1 to
3d2ba2f
Compare
c617f09 to
efdadad
Compare
c15a107 to
747a07d
Compare
dc4396c to
78049b3
Compare
78049b3 to
a8c4ee2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const response: { account_id: string; server_time: number } = { | ||
| account_id: account_id ?? "", | ||
| server_time: Date.now(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include project scope in system.test response
For project‑scoped API keys the hub endpoint system.test returns only an account_id field, because the handler builds a response from account_id alone and discards any project_id injected by authFirst. This means callers cannot distinguish project‑scoped keys (the Python tests expect project_id to be present) and project tokens are effectively indistinguishable from invalid keys. Please include the provided project_id in the response when the key is project‑scoped.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only account-based api keys have access, there is no need for injecting a project_id here. At least, I don't see why. In the future, we'll probably revisit this in more detail, though.
3f34f79 to
5a418c4
Compare
This commit updates the conftest.py test fixtures to properly support both account-scoped and project-scoped API keys during testing. The validate_api_key_config fixture now: - First attempts to detect scope via hub.system.test() (account-scoped keys) - Falls back to project.system.test() if hub fails (project-scoped keys) - Reports both errors if both endpoints fail, for better debugging Also clarifies that the hub endpoint's system.test() is for account-scoped keys only, with clear comments directing project-scoped key users to the project endpoint. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
5a418c4 to
ef5d0ca
Compare
Uh oh!
There was an error while loading. Please reload this page.