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.

Sign up for GitHub

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

Jump to bottom

Api key #1011

Merged
merged 92 commits into from
Mar 29, 2023
Merged

Api key #1011

merged 92 commits into from
Mar 29, 2023

Conversation

dmytrotsko
Copy link
Contributor

@dmytrotsko dmytrotsko commented Oct 27, 2022 •
edited
Loading

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
  • 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

work and others added 30 commits October 18, 2022 09:43
Cleanup unused imports && added role definition and security
c7b0120
@dmytrotsko
Simple api role checking
a700ef2
@dmytrotsko
Render api key warnings
75509cb
@dmytrotsko
Move tests
a796e2c
@dmytrotsko
Add api user ddl
8d48b20
@dmytrotsko
tests
c753f73
@dmytrotsko
improve dockerignore
a15a36e
@dmytrotsko
support api date via env
4fc210d
@dmytrotsko
start with admin interface
ee0c087
@dmytrotsko
start with simple admin interface
0446ea9
@dmytrotsko
very basic admin interface
c7ca927
@dmytrotsko
list all known roles
d066a90
@dmytrotsko
remove log statement
8b09387
@dmytrotsko
fixup tests
e940f8b
@dmytrotsko
add record flag to user
8bb2789
@dmytrotsko
basic record based on flag logging
3161d2e
@dmytrotsko
rename column to tracking
9ab8978
@dmytrotsko
add webhook endpoint for google forms and simplify table
54dc83c
@dmytrotsko
tests for simplified api_user
62122cf
@dmytrotsko
prepare for sending mails with mailgun
32096ac
@dmytrotsko
disable security check for test setup
45414bc
@dmytrotsko
fix tests
60f7cd6
@dmytrotsko
add registered flag
dac3fcb
@dmytrotsko
request a simple api key form with recaptcha check
d880795
@dmytrotsko
implement rate limiting
e76cde4
@dmytrotsko
registered flag not saved
f1f008e
@dmytrotsko
mark all tests no rate limit
98315c6
@dmytrotsko
add link to registration form within api key form
f897e28
@dmytrotsko
elevate sqlalchemy log level to ERROR for test purpose
b345f76
@dmytrotsko
add structlog to requirements
5ce96bd
@dmytrotsko
Moved config variables from _security to _config. Removed duplicated …
4285b87 
…method in _security.py by creating parent class for DBUser and APIUser classes.
nolangormley
nolangormley previously approved these changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@nolangormley nolangormley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took a quick pass of this PR. I think some of the SQLAlchemy ORM usage can be cleaned up, but it is not dire and I do not think it is anything that would slow this system down in a measurable way.

src/server/_security.py Outdated Show resolved Hide resolved
@dmytrotsko
Replaced db.execute delete and update with built-it sqlalchemy functi…
7a364d9 
…ons.
@dmytrotsko
Removed email field. Edited google form link to prefill tracking cons…
e4ae70c 
…ent option as Yes
@dmytrotsko
Removed api_user ddl
c21689d
@dmytrotsko
Changed user roles to use m2m table instead of string in api_user table
1ebff22
@dmytrotsko
Added new variable for api key expiration date
c49c1f8
@dmytrotsko
Updated admin endpoint. Removed email from user details and added cre…
c54412c 
…ation_date, expiration_date and last_api_access_date fields
@dmytrotsko
Updated integration tests
6c50703
@dmytrotsko
Updated endpoints to register user role on endpoint load
3e34e3c
@dmytrotsko
Removed user_role FK in api_user table.
8fbae7d 
Consolidated DBUser and APIUser into 1 class -> APIUser.
@dmytrotsko
Updated according to new structure
95507c2
@dmytrotsko
Removed API_KEY_EXPIRE_AFTER variable
597eeb4
@dmytrotsko
Removed creation_date, expiration_date and last_api_access_date from …
08bd2b6 
…admin UI as we don't have them anymore
@dmytrotsko
Added roles registraion on endpoints load
c436b0a
@dmytrotsko
Removed role registration statement
199d434
@dmytrotsko
Fixed integration tests. Actually reverted latest changes as api_user…
4422a5e 
… table can be truncated again
@dmytrotsko
Added new ddls
806d6e4
@rzats rzats mentioned this pull request Dec 2, 2022
Merged
4 tasks
@korlaxxalrok korlaxxalrok changed the base branch from dev to api-keys March 23, 2023 14:27
@korlaxxalrok korlaxxalrok changed the base branch from api-keys to dev March 23, 2023 14:28
@korlaxxalrok korlaxxalrok changed the base branch from dev to api-keys March 23, 2023 14:28
@krivard krivard merged commit 73efe70 into cmu-delphi:api-keys Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rzats rzats Awaiting requested review from rzats

@melange396 melange396 Awaiting requested review from melange396

@nolangormley nolangormley Awaiting requested review from nolangormley

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dmytrotsko @melange396 @nolangormley @krivard