-
Notifications
You must be signed in to change notification settings - Fork 248
Added OAUTH authentication to backend #1859
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
Added OAUTH authentication to backend #1859
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds OAuth/OIDC authentication support to the Hashtopolis backend, allowing users to exchange OAuth access/identity tokens for application JWT tokens.
Changes:
- Added new
/api/v2/auth/oauth-tokenendpoint for OAuth token exchange - Extracted common token generation logic into
generateTokenForUser()function - Updated JWT token payload to include issuer (iss) and key ID (kid) fields
- Added support for JWKS public key verification
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| src/inc/apiv2/auth/token.routes.php | Implements OAuth token exchange endpoint and refactors token generation logic |
| src/api/v2/index.php | Updates JWT authentication to include key ID in Secret initialization and excludes OAuth endpoint from JWT validation |
| jwks.json.example | Provides example JWKS file with RSA public keys for OAuth token verification |
| docker-compose.postgres.yml | Adds commented volume mount for jwks.json configuration |
| docker-compose.mysql.yml | Adds commented volume mount for jwks.json configuration |
| .gitignore | Excludes jwks.json from version control |
| .devcontainer/docker-compose.postgres.yml | Adds commented volume mount for jwks.json in dev environment |
| .devcontainer/docker-compose.mysql.yml | Adds commented volume mount for jwks.json in dev environment |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!$jwks_file) { | ||
| throw new HttpError("No jwks.json found, upload the jwks public keys to /keys/jwks.json to use OIDC authentication"); | ||
| } | ||
| $jwks = json_decode($jwks_file, true); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing validation for json_decode result. The code doesn't check if json_decode returned null or if the decoded JSON has the expected structure before passing it to JWK::parseKeySet. This could cause errors if the jwks.json file contains invalid JSON or an unexpected structure. Add validation to ensure the JSON is properly decoded and has the expected 'keys' structure.
| $jwks = json_decode($jwks_file, true); | |
| $jwks = json_decode($jwks_file, true); | |
| if ($jwks === null && json_last_error() !== JSON_ERROR_NONE) { | |
| throw new HttpError("Invalid jwks.json: JSON decoding failed (" . json_last_error_msg() . ")"); | |
| } | |
| if (!is_array($jwks) || !isset($jwks['keys']) || !is_array($jwks['keys']) || empty($jwks['keys'])) { | |
| throw new HttpError("Invalid jwks.json format: expected a 'keys' array"); | |
| } |
| $jwt = extractBearerToken($request); | ||
| $decoded_jwt = JWT::decode($jwt, $keys); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OAuth token validation lacks error handling. If the bearer token is null or invalid, this will cause an exception. The code should validate that the token is not null before attempting to decode it.
| throw new HttpError("Token has been expired"); | ||
| } | ||
|
|
||
| $userName = $decoded_jwt->preferred_username; |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The preferred_username claim is accessed without checking if it exists in the decoded token. If the OAuth token does not contain this claim, this will cause an undefined property error. The code should validate that the claim exists before accessing it.
| $app->group("/api/v2/auth/oauth-token", function (RouteCollectorProxy $group) { | ||
|
|
||
| $group->post('', function (Request $request, Response $response, array $args): Response { | ||
| $jwks_file = file_get_contents("/keys/jwks.json"); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded file path "/keys/jwks.json" is not configurable and may not work in all deployment environments. This path should be configurable through environment variables or configuration files to accommodate different deployment scenarios.
| $jwks_file = file_get_contents("/keys/jwks.json"); | ||
| if (!$jwks_file) { | ||
| throw new HttpError("No jwks.json found, upload the jwks public keys to /keys/jwks.json to use OIDC authentication"); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file_get_contents() function can fail and return false for various reasons beyond the file not existing, such as permission issues or I/O errors. The error message should be more generic to cover all failure scenarios, not just missing files.
src/inc/apiv2/auth/token.routes.php
Outdated
| $now = new DateTime(); | ||
| $tokenExpiration = new DateTime("@" . $decoded_jwt->exp); | ||
| if ($now > $tokenExpiration) { | ||
| throw new HttpError("Token has been expired"); | ||
| } |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The token expiration check is redundant since JWT::decode() will automatically verify the token expiration if the 'exp' claim is present. This check happens after decoding, making it unnecessary. Additionally, the error message grammar should be "Token has expired" instead of "Token has been expired".
| if (!$jwks_file) { | ||
| throw new HttpError("No jwks.json found, upload the jwks public keys to /keys/jwks.json to use OIDC authentication"); | ||
| } | ||
| $jwks = json_decode($jwks_file, true); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The json_decode() call does not check if the decoding was successful. If the jwks.json file contains invalid JSON, this will result in $jwks being null, which will then cause an error when passed to JWK::parseKeySet(). The code should check json_last_error() to handle malformed JSON gracefully.
| $decoder = new FirebaseDecoder( | ||
| new Secret($PEPPER[0], 'HS256') | ||
| new Secret($PEPPER[0], 'HS256', hash("sha256", $PEPPER[0])) |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new Secret initialization with three parameters (key, algorithm, kid) is added here but the corresponding change should ensure that the JWT verification process can properly match tokens using the kid (key ID) claim. The implementation should verify that tokens can be validated using the kid field to ensure backward compatibility with existing tokens that may not have this field.
| $token = JWT::encode($payload, $secret, "HS256"); | ||
|
|
||
| $data["token"] = $token; | ||
| $data["expires"] = $future->getTimeStamp(); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The response format for the "expires" field is inconsistent across authentication endpoints. The new OAuth endpoint (line 100) and updated /api/v2/auth/token endpoint (line 123) return a DateTime object, while the /api/v2/auth/refresh endpoint (line 161) still returns a timestamp via getTimeStamp(). For API consistency, all three endpoints should return the same format. Consider updating the refresh endpoint to also return a DateTime object.
| $jwks_file = file_get_contents("/keys/jwks.json"); | ||
| if (!$jwks_file) { | ||
| throw new HttpError("No jwks.json found, upload the jwks public keys to /keys/jwks.json to use OIDC authentication"); | ||
| } | ||
| $jwks = json_decode($jwks_file, true); | ||
| $keys = JWK::parseKeySet($jwks); | ||
| $jwt = extractBearerToken($request); | ||
| $decoded_jwt = JWT::decode($jwt, $keys); | ||
|
|
||
| $now = new DateTime(); | ||
| $tokenExpiration = new DateTime("@" . $decoded_jwt->exp); | ||
| if ($now > $tokenExpiration) { | ||
| throw new HttpError("Token has been expired"); | ||
| } |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OAuth token exchange handler decodes the incoming JWT with JWT::decode($jwt, $keys) and only checks the exp claim, but it does not validate the token’s issuer (iss) or audience (aud). This allows any JWT signed by one of the keys in /keys/jwks.json—including tokens issued for other clients or resources—to be exchanged for a Hashtopolis JWT as long as it contains a preferred_username that matches an existing user. Add strict checks that iss and aud (and, if applicable, azp or similar) match the expected IdP and client before issuing an application token, and reject tokens that do not match.
With this pull request it will become possible to use OAUTH access/identity tokens to exchange for a normal jwt token in hte backend to authenticate.
For this to work the public keys of the tokens should be put in jwks.json, in order to verify the tokens. I have tested the code with keycloak, but it should work with any OAUTH server, aslong as the token has a "preferred_username" claim (which is default in keycloak but can be configured).
To test it you can do the following (for keycloak using the password grant, but should also work for other OAUTH servers and other grant types):
OAUTHTOKEN=$(curl -X POST http://{{url of oauth server}}/realms/master/protocol/openid-connect/token -H "Content-Type: application/x-www-form-urlencoded" -d "grant_type=password" -d "client_id={{CLIENT_ID}}" -d "client_secret={{CLIENT_SECRET}}" -d "username={{USERNAME}}" -d "password={{PASSWORD}}" -d "requested_token_type=urn:ietf:params:oauth:token-type:id_token" | jq -r .access_token)
TOKEN=$(curl -X POST http://localhost:8080/api/v2/auth/oauth-token --compressed --header "Authorization: Bearer $OAUTHTOKEN" | jq -r .token)
And now you can de request as normal since this is the normal jwt token:
curl --compressed --header "Authorization: Bearer $TOKEN" -g 'http://localhost:8080/api/v2/ui/hashtypes?page[size]=5'
Note that currently it is only possible to authenticate as an user where the user with the "preffered_username" claim, already exists in the hashtopolis database. In the future we could perhaps also make it that when the user does not exists, a new user is created, but then the jwt token should also contain which email and access group the user should be in since these are required for new users.
The next steps would be to implement a OAuth 2.0 Authorization Code Grant type of authentication in the frontend to retrieve the OAUTH access/identity token, which can then be used to retrieve a normal JWT token from the backend