Skip to content

Improved API keys logic #1061

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

Merged
merged 113 commits into from
Mar 29, 2023
Merged

Improved API keys logic #1061

merged 113 commits into from
Mar 29, 2023

Conversation

dmytrotsko
Copy link
Contributor

Prerequisites:

  • Unless it is a documentation hotfix it should be merged against the dev branch
  • Branch is up-to-date with the branch to be merged with, i.e. dev
  • Build is successful
  • Code is cleaned up and formatted (Black was used as a code formatter)

Summary

Implements the server logic to require API keys and roles for the endpoints.

List of tasks:

  • check for api key
  • show a soft warning
  • show a hard warning
  • adapt tests
  • admin interface
  • [?] log / track api_key + query if allowed
  • create basic google form
  • connect google form with api server handling -> webhook
  • create a basic request a key form: /admin/create_key
  • use flask-limited for rate limiting
  • setup a Redis DB for shared rate limit tracking
  • add email address validation
  • fix logging (for now we are logging user requests with api_key even without asking user wether he wants to be logged)
  • update documentation ???
  • add tests for admin endpoint
  • define a good default rate limit
  • consider using Redis DB also for account management instead of SQL server

…s which containts all necessary methods to work with User model.
… each test run. This is done because of returning relationship into User model.
@korlaxxalrok korlaxxalrok changed the base branch from dev to api-keys March 23, 2023 17:27
@krivard
Copy link
Contributor

krivard commented Mar 23, 2023

@dmytrotsko do you need to add a redis image to Makefile and ci.yaml?

@dmytrotsko
Copy link
Contributor Author

dmytrotsko commented Mar 24, 2023

@dmytrotsko do you need to add a redis image to Makefile and ci.yaml?

I'm not sure actually about ci.yaml.
But I think I can add it to Makefile as we need it to be built to use API keys stuff.
Or if we have Redis instance somewhere else we just need to provide it in env variables for web-server start.

@krivard
Copy link
Contributor

krivard commented Mar 27, 2023

If we need a redis server in order to pass integration tests, then we need to set one up in ci.yaml

@krivard
Copy link
Contributor

krivard commented Mar 27, 2023

(this PR is not currently running integration tests since it was made from a fork)

@dmytrotsko
Copy link
Contributor Author

If we need a redis server in order to pass integration tests, then we need to set one up in ci.yaml

Okay. I see. Will add it to ci.yaml then

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-advanced-security
Copy link

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@krivard krivard merged commit 717a189 into cmu-delphi:api-keys Mar 29, 2023
@krivard krivard mentioned this pull request Mar 29, 2023
9 tasks
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