Skip to content
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

Deny access to methods other than GET for readonly access tokens #390

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

lfdebrux
Copy link
Member

What problem does this pull request solve?

This PR adds a crude form of access control to the API. Access tokens now have associated permissions; either :all or :readonly. Access tokens with :readonly permissions can only be used for GET requests.

Otherwise, nothing else should be changed with using the API. The aim is to make us able to feel more free as developers about using the API for debugging or inspecting production systems.

Things to consider when reviewing

  • Ensure that you consider the wider context.
  • Does it work when run on your machine?
  • Is it clear what the code is doing?
  • Do the commit messages explain why the changes were made?
  • Are there all the unit tests needed?
  • Has all relevant documentation been updated?

chao-xian and others added 6 commits December 15, 2023 10:46
Running tests threw a `undefined method 'deprecator'` error. This is related to the `time_helpers` from activesupport. This issue is defined here: rails/rails#49495 which suggests this as a workaround rails/rails#49495 (comment)
We aren't using active_storage_blob tables so these would never run.
The prefix breaks the output from Lograge and we are specifying the request id already. [1]
Also moving the stdout as default to application.rb so that it applies to all environments, without needing the env var as we are removing that.

[1]: #369 (comment)
@lfdebrux lfdebrux force-pushed the ldeb-add-access-tokens-permissions branch from 70e6132 to 29af0e6 Compare December 19, 2023 08:39
@lfdebrux lfdebrux force-pushed the ldeb-add-access-tokens-permissions branch 3 times, most recently from 2b52f7a to 8f57543 Compare December 19, 2023 08:50
@lfdebrux lfdebrux force-pushed the ldeb-add-access-tokens-permissions branch from 8f57543 to c09f41e Compare December 19, 2023 08:53
Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

5 New issues
0 Security Hotspots
No data about Coverage
0.8% Duplication on New Code

See analysis details on SonarCloud

@chao-xian chao-xian force-pushed the update-rails-7.1 branch 2 times, most recently from 4e89ec5 to 7d80966 Compare December 19, 2023 16:46
Base automatically changed from update-rails-7.1 to main December 28, 2023 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants