-
Notifications
You must be signed in to change notification settings - Fork 129
Implement support for JWT-based authentication in the Trino Gateway. #812
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
d0c2b8a to
9fb863a
Compare
|
Please add some documentation with an example configuration about the flow in security.md file. Also, mention how to the user mapping works. |
Added the requested documentation - please have a look at when you get a chance. Thank you. |
|
can you please add a more meaningful description as well |
Thank you, @RoeyoOgen. I’ve updated the description to include additional context and details. Please let me know if any further information is needed. |
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 PR implements JWT-based authentication for Trino Gateway, enabling identity propagation from upstream proxies or API gateways. The implementation validates JWT tokens, extracts user identity from configurable claims, and applies user mapping transformations before authorization.
Changes:
- Added JWT authentication support with RSA, HMAC, and JWKS key source options
- Implemented user mapping functionality to transform principals (pattern-based and file-based)
- Extended authentication configuration and webapp to support JWT authentication flow
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| gateway-ha/src/main/java/io/trino/gateway/ha/config/JwtConfiguration.java | Configuration class for JWT authentication settings including key file, issuer, audience, and user mapping |
| gateway-ha/src/main/java/io/trino/gateway/ha/config/AuthenticationConfiguration.java | Extended to include JWT configuration option |
| gateway-ha/src/main/java/io/trino/gateway/ha/security/LbJwtManager.java | Core JWT manager handling token verification for RSA, HMAC, and JWKS key sources |
| gateway-ha/src/main/java/io/trino/gateway/ha/security/LbJwtAuthenticator.java | Authenticator implementing JWT-based authentication with user mapping support |
| gateway-ha/src/main/java/io/trino/gateway/ha/security/UserMapping.java | User mapping implementation supporting pattern and file-based transformations |
| gateway-ha/src/main/java/io/trino/gateway/ha/security/UserMappingException.java | Exception class for user mapping errors |
| gateway-ha/src/main/java/io/trino/gateway/ha/util/JsonUtils.java | JSON parsing utilities supporting various input sources |
| gateway-ha/src/main/java/io/trino/gateway/ha/module/HaGatewayProviderModule.java | Integrated JWT authentication into the module provider |
| gateway-ha/src/main/java/io/trino/gateway/ha/resource/LoginResource.java | Extended login resource to support JWT token retrieval endpoint |
| webapp/src/App.tsx | Updated webapp to handle JWT authentication flow |
| webapp/src/api/webapp/login.ts | Added API method for token retrieval |
| docs/security.md | Comprehensive documentation for JWT authentication and user mapping |
| Test files | Four test classes providing unit and integration test coverage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Prevent multiple initialization runs | ||
| if (access.isAuthorized()) { | ||
| return; | ||
| } |
Copilot
AI
Jan 20, 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 early return in useEffect prevents re-initialization when access state changes. If a user logs out and needs to re-authenticate, this guard will prevent the token initialization from running again. Consider tracking initialization state separately or removing this guard.
| }; | ||
|
|
||
| initializeToken(); | ||
| }, []); // Empty dependency array - only run once on mount |
Copilot
AI
Jan 20, 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 useEffect depends on 'access.isAuthorized()' but 'access' is not included in the dependency array. This violates React's exhaustive-deps rule and could lead to stale closures. Add 'access' to the dependency array.
| }, []); // Empty dependency array - only run once on mount | |
| }, [access]); // Re-run if access instance changes |
| public Response token(@Context HttpHeaders headers) | ||
| { | ||
| String authHeaderVal = headers.getHeaderString(HttpHeaders.AUTHORIZATION); | ||
| String bearerToken = null; | ||
| if (authHeaderVal != null && authHeaderVal.startsWith("Bearer ")) { | ||
| bearerToken = authHeaderVal.substring("Bearer ".length()); | ||
| } | ||
| return Response.ok(Result.ok("Ok", bearerToken)).build(); |
Copilot
AI
Jan 20, 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 endpoint returns the bearer token in the response without validation. This creates a potential security issue where any client can retrieve tokens by simply forwarding the Authorization header. The endpoint should validate the token and potentially return a session token or validate the user's authorization before exposing the JWT.
| public Response token(@Context HttpHeaders headers) | |
| { | |
| String authHeaderVal = headers.getHeaderString(HttpHeaders.AUTHORIZATION); | |
| String bearerToken = null; | |
| if (authHeaderVal != null && authHeaderVal.startsWith("Bearer ")) { | |
| bearerToken = authHeaderVal.substring("Bearer ".length()); | |
| } | |
| return Response.ok(Result.ok("Ok", bearerToken)).build(); | |
| @RolesAllowed({"ADMIN", "USER", "API"}) | |
| public Response token(@Context SecurityContext securityContext) | |
| { | |
| LbPrincipal principal = (LbPrincipal) securityContext.getUserPrincipal(); | |
| if (principal == null) { | |
| throw new WebApplicationException("Unauthorized", Response.Status.UNAUTHORIZED); | |
| } | |
| ImmutableList.Builder<String> rolesBuilder = ImmutableList.builder(); | |
| if (securityContext.isUserInRole("ADMIN")) { | |
| rolesBuilder.add("ADMIN"); | |
| } | |
| if (securityContext.isUserInRole("USER")) { | |
| rolesBuilder.add("USER"); | |
| } | |
| if (securityContext.isUserInRole("API")) { | |
| rolesBuilder.add("API"); | |
| } | |
| List<String> roles = rolesBuilder.build(); | |
| Map<String, Object> resMap = Map.of( | |
| "roles", roles, | |
| "userId", principal.getName(), | |
| "userName", principal.getName()); | |
| return Response.ok(Result.ok("Ok", resMap)).build(); |
| HaGatewayLauncher.main(args); | ||
|
|
||
| // Wait for the gateway to start | ||
| Thread.sleep(5000); |
Copilot
AI
Jan 20, 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.
Using Thread.sleep in tests creates flaky tests and unnecessarily increases test execution time. Consider implementing a proper wait mechanism that polls for gateway readiness or uses a countdown latch.
| export async function getTokenApi(): Promise<any> { | ||
| return api.post('/token', {}) | ||
| } |
Copilot
AI
Jan 20, 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 function name 'getTokenApi' is misleading as it performs a POST request. Consider renaming to 'fetchTokenApi' or 'retrieveTokenApi' to better reflect the HTTP method used.
| export async function getTokenApi(): Promise<any> { | |
| return api.post('/token', {}) | |
| } | |
| export async function fetchTokenApi(): Promise<any> { | |
| return api.post('/token', {}) | |
| } | |
| export const getTokenApi = fetchTokenApi; |
| this.userMappingPattern = Optional.ofNullable(userMappingPattern); | ||
| } | ||
|
|
||
| public Optional<@FileExists File> getUserMappingFile() |
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.
We parse YAML directly and doesn't use airlift's configuration system. The @FileExists won't be effective.
| <groupId>io.airlift</groupId> | ||
| <artifactId>configuration</artifactId> | ||
| </dependency> | ||
|
|
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.
I prefer not to import io.airlift.configuration. We are using YAML directly.
| @@ -0,0 +1,179 @@ | |||
| /* | |||
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.
Looks like this file is from https://github.com/trinodb/trino/blob/2aa8ed4a6be6e9fdf6d6f48c9ab2f3faddbb4adb/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonUtils.java. Can we import it instead of copy?
| import static java.util.Locale.ENGLISH; | ||
| import static java.util.Objects.requireNonNull; | ||
|
|
||
| public final class UserMapping |
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.
When copying code from Trino, please add a comment linking to the original file. This will allow us to pull upstream updates in the future.
|
|
||
| if (log.isDebugEnabled()) { | ||
| unverifiedJwt.getClaims().forEach( | ||
| (key, value) -> log.debug("JWT %s : %s", key, value)); |
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.
I know this is convenient for debugging, but printing a JWT in the debug logs isn’t a good idea and creates a security risk.
Description
Add JWT (JSON Web Token) authentication support to Trino Gateway, enabling identity propagation from upstream proxies or API gateways.
Motivation
In many enterprise deployments, Trino Gateway sits behind a reverse proxy or API gateway (e.g., Nginx, Istio, Envoy, Kong, or a custom proxy) that handles user authentication. The proxy authenticates users against an Identity Provider (IdP) such as Azure AD, Okta, or Keycloak, which issues a JWT token. The proxy then forwards requests to Trino Gateway with the token in the
Authorization: Bearer <jwt-token>header.Currently, Trino Gateway cannot consume these forwarded JWT tokens for user identification. This PR enables Trino Gateway to:
Changes
JWT Authentication (
LbJwtAuthenticator,LbJwtManager)Authorization: Bearerheaderiss) and audience (aud)sub)User Mapping (
UserMapping)Transforms the JWT principal to an internal username before authorization:
(.*)@company\.comextracts username from emailConfiguration
Files Changed
JwtConfiguration,AuthenticationConfigurationLbJwtAuthenticator,LbJwtManagerUserMapping,UserMappingExceptionJsonUtilsHaGatewayProviderModule,LoginResourcedocs/security.mdHow It Works
Authorization: Bearer <jwt>suboremail)Testing
Additional context and related issues
#449
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required, with the following suggested text:
* Fix some things.