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

removed ALLOW_ANY_API_KEY and allow skipping API key check in debug #61

Merged
merged 2 commits into from
Dec 25, 2022

Conversation

yk
Copy link
Collaborator

@yk yk commented Dec 25, 2022

the new environment variable is called DEBUG_SKIP_API_KEY_CHECK

@yk yk linked an issue Dec 25, 2022 that may be closed by this pull request
if api_key is not None:
if settings.ALLOW_ANY_API_KEY:
if api_key is not None or settings.DEBUG_SKIP_API_KEY_CHECK:
if settings.DEBUG_SKIP_API_KEY_CHECK:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The settings.DEBUG_SKIP_API_KEY_CHECK: was put below the api_key is not None check to give the dev a way to see the auth-failure case when no api-key was sent with the request. I personally think it is valuable to able to 'test' this case - even though it requires you to send a random api key. But that is debatable of course.

We might move the whole if settings.DEBUG_SKIP_API_KEY_CHECK: block above the if api_key check to simplify things.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point. I've re-introduced it, so there are now both variables to handle both cases. I've put the ALLOW_ANY into the ansible playbook so that against the dev machine, at least the presence of API keys is validated, but in local development, I've activated the complete skip.

Copy link
Collaborator

@andreaskoepf andreaskoepf left a comment

Choose a reason for hiding this comment

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

ok, we probably need to document the settings at a later point (e.g. with some doc-strings, since with the two options there will be now more questions than before ;-)) but otherwise lgtm.

@yk yk merged commit c3a6226 into main Dec 25, 2022
@yk yk deleted the 59-document-how-to-develop-on-the-backend-api branch December 25, 2022 22:47
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.

Document how to develop on the backend API
2 participants