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

PB-1366: make new auth work for write requests. #502

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

adk-swisstopo
Copy link
Member

Django is configured to use CSRF mitigation tokens. This only works for clients that are web browsers. Typical API clients are not web browsers, do not keep track of cookies and fail CSRF mitigation checks. The old token authentication method works around that by disabling CSRF mitigation when that authentication method is used. With this change, we do the same for the new token authentication method.

In PB-1009 we implemented the new authentication method for the Admin UI. This change does the same for the API endpoints and add tests exercising it.

The API endpoints are rest_framework views that rely on authentication methods defined by the REST framework which is different from the Admin UI authentication plumbing. To support both, we need to have modules for both even though they basically do the same thing.

Specifically:

  • create api_gateway_authentication as rest_framework authentication module
  • create api_gateway_middleware as generic Django authentication module (for Admin UI)
  • abstract the common parts into a new api_gateway module
  • configure both authentication modules in the debug build
  • add tests exercising a PUT API endpoint for both v0.9 and v1, including CSRF checks

This is all flag-guarded behind FEATURE_AUTH_ENABLE_APIGW.

@github-actions github-actions bot added the bug label Jan 23, 2025
@adk-swisstopo adk-swisstopo force-pushed the fix-PB-1366-auth-write branch from d1108d8 to 09ac94a Compare January 23, 2025 15:05
@adk-swisstopo adk-swisstopo requested review from msom and benschs January 23, 2025 15:06
Django is configured to use CSRF mitigation tokens. This only works for clients
that are web browsers. Typical API clients are not web browsers, do not keep
track of cookies and fail CSRF mitigation checks. The old token authentication
method works around that by disabling CSRF mitigation when that authentication
method is used. With this change, we do the same for the new token authentication
method.

In PB-1009 we implemented the new authentication method for the Admin UI.
This change does the same for the API endpoints and add tests exercising it.

The API endpoints are rest_framework views that rely on authentication
methods defined by the REST framework which is different from the Admin UI
authentication plumbing. To support both, we need to have modules for both
even though they basically do the same thing.

Specifically:
* create `api_gateway_authentication` as `rest_framework` authentication module
* create `api_gateway_middleware` as generic Django authentication module (for Admin UI)
* abstract the common parts into a new `api_gateway` module
* configure both authentication modules in the debug build
* add tests exercising a PUT API endpoint for both v0.9 and v1, including CSRF checks

This is all flag-guarded behind FEATURE_AUTH_ENABLE_APIGW.
@adk-swisstopo adk-swisstopo force-pushed the fix-PB-1366-auth-write branch from 09ac94a to d90b71f Compare January 23, 2025 16:21
Copy link
Contributor

@boecklic boecklic left a comment

Choose a reason for hiding this comment

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

Very nice!

@adk-swisstopo adk-swisstopo merged commit 4a1303d into develop Jan 27, 2025
3 checks passed
@adk-swisstopo adk-swisstopo deleted the fix-PB-1366-auth-write branch January 27, 2025 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants