-
Notifications
You must be signed in to change notification settings - Fork 3.2k
feat(): Adding authenticator for OIDC OAuth #14707
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: master
Are you sure you want to change the base?
feat(): Adding authenticator for OIDC OAuth #14707
Conversation
Adding docs Adding docs
} | ||
|
||
private PublicKey loadPublicKey(String jwksUri, String keyId) throws Exception { | ||
HttpRequest request = HttpRequest.newBuilder().uri(URI.create(jwksUri)).build(); |
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.
HTTP request might enable SSRF attack - high severity
If an attacker can control the URL input leading into this http request, the attack might be able to perform an SSRF attack. This kind of attack is even more dangerous is the application returns the result of the URL fetch to the user. It can serve as an initial access point for an attacker for stealing credentials in the cloud.
Remediation: If possible, only allow requests to verified domains. If not, consult the article linked above to learn about other mitigating techniques such as disabling redirects, blocking private IPs and making sure private services have internal authentication. If you return data coming from the request to the user, validate the data before returning it to make sure you don't return random data.
View details in Aikido Security
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.
Since this is a deployment time setting and not pulled from a user request, users cannot supply this. The recommendation that production settings should use static provider addresses this risk.
Bundle ReportChanges will decrease total bundle size by 2.47kB (-0.01%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: datahub-react-web-esmAssets Changed:
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
} | ||
|
||
private PublicKey loadPublicKey(String jwksUri, String keyId) throws Exception { | ||
HttpRequest request = HttpRequest.newBuilder().uri(URI.create(jwksUri)).build(); |
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.
Since this is a deployment time setting and not pulled from a user request, users cannot supply this. The recommendation that production settings should use static provider addresses this risk.
metadata-service/configuration/src/main/resources/application.yaml
Outdated
Show resolved
Hide resolved
...auth-impl/src/main/java/com/datahub/authentication/token/DataHubOAuthSigningKeyResolver.java
Show resolved
Hide resolved
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.
LGTM with a few changes requested before merging.
Addressing comments. Thank you! |
} | ||
|
||
private PublicKey loadPublicKey(String jwksUri, String keyId, String algorithm) throws Exception { | ||
HttpRequest request = HttpRequest.newBuilder().uri(URI.create(jwksUri)).build(); |
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.
HTTP request might enable SSRF attack - high severity
If an attacker can control the URL input leading into this http request, the attack might be able to perform an SSRF attack. This kind of attack is even more dangerous is the application returns the result of the URL fetch to the user. It can serve as an initial access point for an attacker for stealing credentials in the cloud.
Remediation: If possible, only allow requests to verified domains. If not, consult the article linked above to learn about other mitigating techniques such as disabling redirects, blocking private IPs and making sure private services have internal authentication. If you return data coming from the request to the user, validate the data before returning it to make sure you don't return random data.
View details in Aikido Security
# External OAuth Configuration | ||
- EXTERNAL_OAUTH_ENABLED=true | ||
- EXTERNAL_OAUTH_TRUSTED_ISSUERS=https://my-okta-domain.okta.com/oauth2/default | ||
- EXTERNAL_OAUTH_ALLOWED_AUDIENCES=0oa1234567890abcdef |
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.
Exposed secret in docs/authentication/external-oauth-providers.md - low severity
Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
View details in Aikido Security
In this PR, we add support for an OIDC Oauth authenticator that allows customers to use service accounts to access DataHub.
Specifically:
Validated this against Okta Service Account APIs. In the next PR, I'll provide some more details about how to configure okta service accounts to use with this.
Testing
Confirmed by setting up a test okta account with Service Account inside: