Conversation
|
@jamesfitzer I can't add you as a reviewer until you accept the invite to collaborate, but would you mind also taking a look when you get a chance? |
|
Ok should be good now |
jamesfitzer
left a comment
jamesfitzer
left a comment
There was a problem hiding this comment.
Other than toadzky's comment, I don't see any issues here. If nobody is denied right now, the migration issue might be superfluous but it's nice to do it in the interests of correctness/completeness.
Nice clean single issue PR. Good work
|
|
||
| def upgrade(): | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.alter_column('key', 'blacklisted', new_column_name='denied') |
There was a problem hiding this comment.
I tested out upgrading and downgrading with a denied key, and it kept the attribute both ways with this, so I think we're good on that now.
aaron-junot
force-pushed
the
ditch-racism
branch
from
4b16ac8 to
8aab65c
Compare
|
Reviews were dismissed because I had to rebase to get back up to date with |
4 participants
The word
blacklistisn't a very inclusive term. I think thatdenylistis a much better term to use, and besides, it's more descriptive for what we're doing to the API key.This PR involves a DB migration and also has everything to do with our authentication, so please give it a solid review. To run the migration locally, you should be able to run
make migrate. Let me know if this doesn't work, because we'll want to fix that command. And of course, let me know if you run into any issues.Testing:
/apikeyendpointPOST /resourcesPUT /resourcesdocker-compose run resources-api flask apikey deny <email address>(can also pass in API key instead of email address)/apikeyendpoint after it's been put on the denylistPOSTorPUTrequest to the/resourcesendpoint with your denied API keydocker-compose run resources-api flask apikey reactivate <email address>(can also pass in API key instead of email address)