From 861b93c30d6dfaaef2003ae39218223b3509c716 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Dec 2022 18:00:47 +0100 Subject: [PATCH 01/12] feat(api): first draft to enable OIDC bearer token API access #9229 --- .../iq/dataverse/api/AbstractApiBean.java | 90 ++++++++++++++++++- 1 file changed, 89 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index e919ecf786d..c0bc8569f7d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -1,5 +1,12 @@ package edu.harvard.iq.dataverse.api; +import com.nimbusds.oauth2.sdk.ErrorObject; +import com.nimbusds.oauth2.sdk.ParseException; +import com.nimbusds.oauth2.sdk.http.HTTPResponse; +import com.nimbusds.oauth2.sdk.token.BearerAccessToken; +import com.nimbusds.openid.connect.sdk.UserInfoRequest; +import com.nimbusds.openid.connect.sdk.UserInfoResponse; +import com.nimbusds.openid.connect.sdk.claims.UserInfo; import edu.harvard.iq.dataverse.DataFile; import edu.harvard.iq.dataverse.DataFileServiceBean; import edu.harvard.iq.dataverse.Dataset; @@ -25,10 +32,13 @@ import edu.harvard.iq.dataverse.UserNotificationServiceBean; import edu.harvard.iq.dataverse.UserServiceBean; import edu.harvard.iq.dataverse.actionlogging.ActionLogServiceBean; +import edu.harvard.iq.dataverse.authorization.AuthenticationProvider; import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; import edu.harvard.iq.dataverse.authorization.DataverseRole; import edu.harvard.iq.dataverse.authorization.RoleAssignee; import edu.harvard.iq.dataverse.authorization.groups.GroupServiceBean; +import edu.harvard.iq.dataverse.authorization.providers.oauth2.OAuth2Exception; +import edu.harvard.iq.dataverse.authorization.providers.oauth2.oidc.OIDCAuthProvider; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.GuestUser; import edu.harvard.iq.dataverse.authorization.users.PrivateUrlUser; @@ -54,11 +64,16 @@ import edu.harvard.iq.dataverse.util.json.JsonParser; import edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder; import edu.harvard.iq.dataverse.validation.PasswordValidatorServiceBean; + +import java.io.IOException; import java.io.StringReader; import java.net.URI; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; +import java.util.List; +import java.util.Optional; import java.util.UUID; import java.util.concurrent.Callable; import java.util.logging.Level; @@ -78,6 +93,7 @@ import javax.persistence.PersistenceContext; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Context; +import javax.ws.rs.core.HttpHeaders; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.ResponseBuilder; @@ -92,6 +108,7 @@ public abstract class AbstractApiBean { private static final Logger logger = Logger.getLogger(AbstractApiBean.class.getName()); private static final String DATAVERSE_KEY_HEADER_NAME = "X-Dataverse-key"; + private static final String OIDC_AUTH_SCHEME = "Bearer"; private static final String PERSISTENT_ID_KEY=":persistentId"; private static final String ALIAS_KEY=":alias"; public static final String STATUS_ERROR = "ERROR"; @@ -426,6 +443,19 @@ private AuthenticatedUser findAuthenticatedUserOrDie( String key, String wfid ) if (authUser != null) { return authUser; } + // TODO: Add feature flag barrier here! + } else if (getOidcBearerToken(httpRequest).isPresent()) { + UserInfo userInfo = verifyOidcBearerToken(getOidcBearerToken(httpRequest).get()); + + // TODO: Only usable for OIDC users for now, just look it up via the subject. + // This will need to be modified to provide mappings somehow for existing non-OIDC-users. + AuthenticatedUser authUser = authSvc.getAuthenticatedUser(userInfo.getSubject().getValue()); + + // TODO: this is code dup par excellence. Needs refactoring. Maybe fine for Proof-of-Concept. + if (authUser != null) { + authUser = userSvc.updateLastApiUseTime(authUser); + return authUser; + } } //Just send info about the apiKey - workflow users will learn about invocationId elsewhere throw new WrappedResponse(badApiKey(null)); @@ -451,7 +481,65 @@ private AuthenticatedUser getAuthenticatedUserFromSignedUrl() { } return authUser; } - + + /** + * Retrieve the raw, encoded token value from the Authorization Bearer HTTP header as defined in RFC 6750 + * @param request The HTTP request coming in + * @return An {@link Optional} either empty if not present or the raw token from the header + */ + Optional getOidcBearerToken(HttpServletRequest request) { + String authHeader = request.getHeader(HttpHeaders.AUTHORIZATION); + + if (authHeader != null && authHeader.toLowerCase().startsWith(OIDC_AUTH_SCHEME.toLowerCase() + " ")) { + return Optional.of(authHeader.substring(OIDC_AUTH_SCHEME.length()+1)); + } else { + return Optional.empty(); + } + } + + + /** + *

Verify an OIDC access token by dealing the access token for a UserInfo object from the provider

+ *

+ * TODO: This is a proof of concept, providing value for IQSS#9229 and first steps for our SPA move. It ... + * - will need more tweaks (see inline comments), + * - should be extended to support JWT access tokens to avoid the extra detour to the OIDC provider, + * - will need a feature flag activation either here or in calling code, + * - needs to be moved to a distinct place when we head for authentication filters in future iterations. + *

+ * + * @param token The string containing the encoded JWT + * @return + */ + UserInfo verifyOidcBearerToken(String token) throws WrappedResponse { + try { + BearerAccessToken accessToken = BearerAccessToken.parse(token); + + // Retrieve data of the user accessing the API from the provider. + // No need to introspect the token here, the userInfoRequest also validates the token, as the provider + // is the source of truth. + // TODO: retrieve the userinfo endpoint from the OIDC provider already present via AuthenticationServiceBean + HTTPResponse response = new UserInfoRequest(URI.create("http://localhost:8090/realms/master/protocol/openid-connect/userinfo"), accessToken) + .toHTTPRequest() + .send(); + + UserInfoResponse infoResponse = UserInfoResponse.parse(response); + + // If error, throw 401 error exception + if (! infoResponse.indicatesSuccess() ) { + ErrorObject error = infoResponse.toErrorResponse().getErrorObject(); + // TODO: make the response more explaining? + throw new WrappedResponse(error(Status.UNAUTHORIZED, "Could not retrieve user info from provider")); + } + + // Success --> return info + return infoResponse.toSuccessResponse().getUserInfo(); + } catch (ParseException | IOException e) { + // TODO: make the response more explaining + throw new WrappedResponse(error(Status.UNAUTHORIZED, "Could not retrieve user info from provider")); + } + } + protected Dataverse findDataverseOrDie( String dvIdtf ) throws WrappedResponse { Dataverse dv = findDataverse(dvIdtf); if ( dv == null ) { From f5862e30cbabbba954636c8e3cb482515996f088 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Dec 2022 19:48:15 +0100 Subject: [PATCH 02/12] feat(settings): add feature gates functionality for the first time #9229 #7000 --- .../iq/dataverse/settings/FeatureGates.java | 40 +++++++++++++++++++ .../iq/dataverse/settings/JvmSettings.java | 4 ++ 2 files changed, 44 insertions(+) create mode 100644 src/main/java/edu/harvard/iq/dataverse/settings/FeatureGates.java diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/FeatureGates.java b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureGates.java new file mode 100644 index 00000000000..c2f3e3aea89 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureGates.java @@ -0,0 +1,40 @@ +package edu.harvard.iq.dataverse.settings; + +/** + *

This enum holds so-called "feature gates" aka "feature flags". It can be used throughout the application + * to avoid activating or using experimental functionality or feature previews that are opt-in.

+ * + *

The current implementation reuses {@link JvmSettings} to interpret any + * boolean values + * (true == case-insensitive one of "true", "1", "YES", "Y", "ON") and hook into the usual settings system + * (any MicroProfile Config Source available).

+ * + * If you add any new gates, please add a setting in JvmSettings, think of a default status, add some Javadocs + * about the gated feature and add a "@since" tag to make it easier to identify when a gate has been introduced. + * + */ +public enum FeatureGates { + + /** + * Enabling will unblock access to the API with an OIDC access token in addition to other available methods. + * @apiNote Open gate by setting "dataverse.feature.api-oidc-access" + * @since Dataverse 5.13 + * @see JvmSettings#GATE_API_OIDC_ACCESS + */ + API_OIDC_ACCESS(JvmSettings.GATE_API_OIDC_ACCESS, false), + + ; + + final JvmSettings setting; + final boolean defaultStatus; + + FeatureGates(JvmSettings setting, boolean defaultStatus) { + this.setting = setting; + this.defaultStatus = defaultStatus; + } + + public boolean enabled() { + return setting.lookupOptional(Boolean.class).orElse(defaultStatus); + } + +} diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java index e409607346b..4bf4fc9bf24 100644 --- a/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java +++ b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java @@ -46,6 +46,10 @@ public enum JvmSettings { SCOPE_API(PREFIX, "api"), API_SIGNING_SECRET(SCOPE_API, "signing-secret"), + // FEATURE GATES SETTINGS + SCOPE_GATES(PREFIX, "feature"), + GATE_API_OIDC_ACCESS(SCOPE_GATES, "api-oidc-access"), + ; private static final String SCOPE_SEPARATOR = "."; From 04b7eac7dba67d7f594d39dfcae4888ebcf17255 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Dec 2022 19:49:12 +0100 Subject: [PATCH 03/12] feat(api): add OIDC API access feature gate to authentication #9229 --- .../java/edu/harvard/iq/dataverse/api/AbstractApiBean.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index c0bc8569f7d..f9d3868eef2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -56,6 +56,7 @@ import edu.harvard.iq.dataverse.privateurl.PrivateUrlServiceBean; import edu.harvard.iq.dataverse.locality.StorageSiteServiceBean; import edu.harvard.iq.dataverse.search.savedsearch.SavedSearchServiceBean; +import edu.harvard.iq.dataverse.settings.FeatureGates; import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.BundleUtil; @@ -443,8 +444,8 @@ private AuthenticatedUser findAuthenticatedUserOrDie( String key, String wfid ) if (authUser != null) { return authUser; } - // TODO: Add feature flag barrier here! - } else if (getOidcBearerToken(httpRequest).isPresent()) { + + } else if (FeatureGates.API_OIDC_ACCESS.enabled() && getOidcBearerToken(httpRequest).isPresent()) { UserInfo userInfo = verifyOidcBearerToken(getOidcBearerToken(httpRequest).get()); // TODO: Only usable for OIDC users for now, just look it up via the subject. From 3f900e5c8f729acba86d56600f7dfe6da5e82c1f Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Dec 2022 19:50:10 +0100 Subject: [PATCH 04/12] chore(api): remove unused imports from AbstractApiBean #9229 --- .../java/edu/harvard/iq/dataverse/api/AbstractApiBean.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index f9d3868eef2..8614df38ad0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -32,13 +32,10 @@ import edu.harvard.iq.dataverse.UserNotificationServiceBean; import edu.harvard.iq.dataverse.UserServiceBean; import edu.harvard.iq.dataverse.actionlogging.ActionLogServiceBean; -import edu.harvard.iq.dataverse.authorization.AuthenticationProvider; import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; import edu.harvard.iq.dataverse.authorization.DataverseRole; import edu.harvard.iq.dataverse.authorization.RoleAssignee; import edu.harvard.iq.dataverse.authorization.groups.GroupServiceBean; -import edu.harvard.iq.dataverse.authorization.providers.oauth2.OAuth2Exception; -import edu.harvard.iq.dataverse.authorization.providers.oauth2.oidc.OIDCAuthProvider; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.GuestUser; import edu.harvard.iq.dataverse.authorization.users.PrivateUrlUser; @@ -69,11 +66,8 @@ import java.io.IOException; import java.io.StringReader; import java.net.URI; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.Enumeration; -import java.util.List; import java.util.Optional; import java.util.UUID; import java.util.concurrent.Callable; From 5e851433c0f4c0bcd379ec2943445e73b69023c2 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Dec 2022 20:48:13 +0100 Subject: [PATCH 05/12] refactor(settings,api): rename feature gates to flags as requested by @pdurbin #9229 --- .../iq/dataverse/api/AbstractApiBean.java | 4 ++-- .../{FeatureGates.java => FeatureFlags.java} | 16 ++++++++-------- .../iq/dataverse/settings/JvmSettings.java | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) rename src/main/java/edu/harvard/iq/dataverse/settings/{FeatureGates.java => FeatureFlags.java} (64%) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 8614df38ad0..9c70ffd4d3c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -53,7 +53,7 @@ import edu.harvard.iq.dataverse.privateurl.PrivateUrlServiceBean; import edu.harvard.iq.dataverse.locality.StorageSiteServiceBean; import edu.harvard.iq.dataverse.search.savedsearch.SavedSearchServiceBean; -import edu.harvard.iq.dataverse.settings.FeatureGates; +import edu.harvard.iq.dataverse.settings.FeatureFlags; import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.BundleUtil; @@ -439,7 +439,7 @@ private AuthenticatedUser findAuthenticatedUserOrDie( String key, String wfid ) return authUser; } - } else if (FeatureGates.API_OIDC_ACCESS.enabled() && getOidcBearerToken(httpRequest).isPresent()) { + } else if (FeatureFlags.API_OIDC_ACCESS.enabled() && getOidcBearerToken(httpRequest).isPresent()) { UserInfo userInfo = verifyOidcBearerToken(getOidcBearerToken(httpRequest).get()); // TODO: Only usable for OIDC users for now, just look it up via the subject. diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/FeatureGates.java b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java similarity index 64% rename from src/main/java/edu/harvard/iq/dataverse/settings/FeatureGates.java rename to src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java index c2f3e3aea89..8d89efd6608 100644 --- a/src/main/java/edu/harvard/iq/dataverse/settings/FeatureGates.java +++ b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java @@ -1,7 +1,7 @@ package edu.harvard.iq.dataverse.settings; /** - *

This enum holds so-called "feature gates" aka "feature flags". It can be used throughout the application + *

This enum holds so-called "feature flags" aka "feature gates", etc. It can be used throughout the application * to avoid activating or using experimental functionality or feature previews that are opt-in.

* *

The current implementation reuses {@link JvmSettings} to interpret any @@ -9,26 +9,26 @@ * (true == case-insensitive one of "true", "1", "YES", "Y", "ON") and hook into the usual settings system * (any MicroProfile Config Source available).

* - * If you add any new gates, please add a setting in JvmSettings, think of a default status, add some Javadocs - * about the gated feature and add a "@since" tag to make it easier to identify when a gate has been introduced. + * If you add any new flags, please add a setting in JvmSettings, think of a default status, add some Javadocs + * about the flagged feature and add a "@since" tag to make it easier to identify when a flag has been introduced. * */ -public enum FeatureGates { +public enum FeatureFlags { /** * Enabling will unblock access to the API with an OIDC access token in addition to other available methods. - * @apiNote Open gate by setting "dataverse.feature.api-oidc-access" + * @apiNote Raise flag by setting "dataverse.feature.api-oidc-access" * @since Dataverse 5.13 - * @see JvmSettings#GATE_API_OIDC_ACCESS + * @see JvmSettings#FLAG_API_OIDC_ACCESS */ - API_OIDC_ACCESS(JvmSettings.GATE_API_OIDC_ACCESS, false), + API_OIDC_ACCESS(JvmSettings.FLAG_API_OIDC_ACCESS, false), ; final JvmSettings setting; final boolean defaultStatus; - FeatureGates(JvmSettings setting, boolean defaultStatus) { + FeatureFlags(JvmSettings setting, boolean defaultStatus) { this.setting = setting; this.defaultStatus = defaultStatus; } diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java index 4bf4fc9bf24..2023f2df6e1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java +++ b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java @@ -46,9 +46,9 @@ public enum JvmSettings { SCOPE_API(PREFIX, "api"), API_SIGNING_SECRET(SCOPE_API, "signing-secret"), - // FEATURE GATES SETTINGS - SCOPE_GATES(PREFIX, "feature"), - GATE_API_OIDC_ACCESS(SCOPE_GATES, "api-oidc-access"), + // FEATURE FLAGS SETTINGS + SCOPE_FLAGS(PREFIX, "feature"), + FLAG_API_OIDC_ACCESS(SCOPE_FLAGS, "api-oidc-access"), ; From b11e4f1a72c8205dff9e93a9da721c84ede1913f Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Dec 2022 21:45:15 +0100 Subject: [PATCH 06/12] doc(dev,auth): add initial feature flags documentation to the guides #9229 #9223 --- .../source/developers/configuration.rst | 14 ++++++++++ .../source/installation/config.rst | 28 +++++++++++++++++++ .../source/installation/oidc.rst | 6 ++++ 3 files changed, 48 insertions(+) diff --git a/doc/sphinx-guides/source/developers/configuration.rst b/doc/sphinx-guides/source/developers/configuration.rst index fb15fea7900..4f550e59f90 100644 --- a/doc/sphinx-guides/source/developers/configuration.rst +++ b/doc/sphinx-guides/source/developers/configuration.rst @@ -109,3 +109,17 @@ always like ``dataverse..newname...=old.property.name``. Note this d aliases. Details can be found in ``edu.harvard.iq.dataverse.settings.source.AliasConfigSource`` + +Adding a Feature Flag +^^^^^^^^^^^^^^^^^^^^^ + +Some parts of our codebase might be opt-in only. Experimental or optional features can be switched on using our +usual configuration mechanism, a JVM setting. + +Feature flags are implemented in the enumeration ``edu.harvard.iq.dataverse.settings.FeatureFlags``, which allows for +convenient usage of it anywhere in the codebase. When adding a flag, please add a setting in ``JvmSettings`` (see above, +also mind to use the appropriate scope ``dataverse.feature``), think of a default status, add some Javadocs about the +flagged feature and add a ``@since`` tag to make it easier to identify when a flag has been introduced. + +We want to maintain a list of all :ref:`feature flags ` in the :ref:`configuration guide `, +please add yours to the list. \ No newline at end of file diff --git a/doc/sphinx-guides/source/installation/config.rst b/doc/sphinx-guides/source/installation/config.rst index 2c576b03989..a2142578459 100644 --- a/doc/sphinx-guides/source/installation/config.rst +++ b/doc/sphinx-guides/source/installation/config.rst @@ -1769,6 +1769,34 @@ production context! Rely on password alias, secrets directory or cloud based sou +.. _feature-flags: + +Feature Flags +------------- + +Certain features might be deactivated because they are experimental and/or opt-in. If you want to enable these, please +find all known feature flags below. Any of these flags can be activated using a boolean value +(case-insensitive, one of "true", "1", "YES", "Y", "ON") for the setting. + +.. list-table:: + :widths: 35 50 15 + :header-rows: 1 + :align: left + + * - Flag Name + - Description + - Default status + * - ``dataverse.feature.api-oidc-access`` + - When using an :doc:`OIDC authentication provider `, also enable using access tokens from it for API + authentication. Useful to integrate services or SPAs with the API when using cross-service logins. + Not usable for users from other authentication providers! + - Disabled + +**Note:** Can be set via any `supported MicroProfile Config API source`_, e.g. the environment variable +``DATAVERSE_FEATURE_API_OIDC_ACCESS``. + + + .. _:ApplicationServerSettings: Application Server Settings diff --git a/doc/sphinx-guides/source/installation/oidc.rst b/doc/sphinx-guides/source/installation/oidc.rst index a40ef758dc7..42c7cc2e7fc 100644 --- a/doc/sphinx-guides/source/installation/oidc.rst +++ b/doc/sphinx-guides/source/installation/oidc.rst @@ -87,3 +87,9 @@ configuration option. For details, see :doc:`config`. In contrast to our :doc:`oauth2`, you can use multiple providers by creating distinct configurations enabled by the same technology and without modifying the Dataverse Software code base (standards for the win!). +Limitations +----------- + +Before Dataverse release 5.13, there was no option to use an Open ID Connect authentication provider with +the "Authentication Code Flow" to access the API. As of this version, there is builtin experimental support for this. +Please see the corresponding :ref:`feature flag ` to enable it. \ No newline at end of file From dd4310ba9e4a5475a49f294f97c9f55d1b04570b Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Dec 2022 22:24:54 +0100 Subject: [PATCH 07/12] refactor(auth): cleanup OIDCAuthProvider + expose UserInfo URI #9229 - Expose the UserInfo endpoint URI from the OIDC provider metadata - Not exposing the complete metadata on purpose to keep it non-modifiable and sealed inside the provider instance - Minor method renaming to better explain what the method is about - Fix missing newlines for single line method --- .../oauth2/oidc/OIDCAuthProvider.java | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/oidc/OIDCAuthProvider.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/oidc/OIDCAuthProvider.java index a9c44010950..01dbbb649b2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/oidc/OIDCAuthProvider.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/oidc/OIDCAuthProvider.java @@ -62,7 +62,7 @@ public OIDCAuthProvider(String aClientId, String aClientSecret, String issuerEnd this.clientSecret = aClientSecret; // nedded for state creation this.clientAuth = new ClientSecretBasic(new ClientID(aClientId), new Secret(aClientSecret)); this.issuer = new Issuer(issuerEndpointURL); - getMetadata(); + setupMetadata(); } /** @@ -74,7 +74,9 @@ public OIDCAuthProvider(String aClientId, String aClientSecret, String issuerEnd * @return false */ @Override - public boolean isDisplayIdentifier() { return false; } + public boolean isDisplayIdentifier() { + return false; + } /** * Setup metadata from OIDC provider during creation of the provider representation @@ -82,9 +84,9 @@ public OIDCAuthProvider(String aClientId, String aClientSecret, String issuerEnd * @throws IOException when sth. goes wrong with the retrieval * @throws ParseException when the metadata is not parsable */ - void getMetadata() throws AuthorizationSetupException { + void setupMetadata() throws AuthorizationSetupException { try { - this.idpMetadata = getMetadata(this.issuer); + this.idpMetadata = fetchMetadata(this.issuer); } catch (IOException ex) { logger.severe("OIDC provider metadata at \"+issuerEndpointURL+\" not retrievable: "+ex.getMessage()); throw new AuthorizationSetupException("OIDC provider metadata at "+this.issuer.getValue()+" not retrievable."); @@ -106,7 +108,7 @@ void getMetadata() throws AuthorizationSetupException { * @throws IOException when sth. goes wrong with the retrieval * @throws ParseException when the metadata is not parsable */ - OIDCProviderMetadata getMetadata(Issuer issuer) throws IOException, ParseException { + OIDCProviderMetadata fetchMetadata(Issuer issuer) throws IOException, ParseException { // Will resolve the OpenID provider metadata automatically OIDCProviderConfigurationRequest request = new OIDCProviderConfigurationRequest(issuer); @@ -118,6 +120,18 @@ OIDCProviderMetadata getMetadata(Issuer issuer) throws IOException, ParseExcepti return OIDCProviderMetadata.parse(httpResponse.getContentAsJSONObject()); } + + /** + * Retrieve the user info endpoint of this provider, used by the OIDC API access feature to use access tokens. + * Returned URI is immutable and will not harm the provider setup. + * + * @return The {@link URI} of the UserInfo endpoint. + */ + public URI getUserInfoEndpointURI() { + return this.idpMetadata.getUserInfoEndpointURI(); + } + + /** * TODO: remove when refactoring package and {@link AbstractOAuth2AuthenticationProvider} */ From 3694d8119b6ba977c7aa6459c3d5a78b72fe3171 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Dec 2022 22:28:14 +0100 Subject: [PATCH 08/12] feat(api): query all avail OIDC providers for user info #9229 Replace the placeholder of static endpoint with retrieving the UserInfo endpoint URI from all know OIDC authentication providers and iterate over them. The access token might match for any of them. Also making the errors a bit more descriptive and adding logging. --- .../iq/dataverse/api/AbstractApiBean.java | 67 +++++++++++++------ 1 file changed, 46 insertions(+), 21 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 9c70ffd4d3c..e9bd06323ca 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -32,10 +32,12 @@ import edu.harvard.iq.dataverse.UserNotificationServiceBean; import edu.harvard.iq.dataverse.UserServiceBean; import edu.harvard.iq.dataverse.actionlogging.ActionLogServiceBean; +import edu.harvard.iq.dataverse.authorization.AuthenticationProvider; import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; import edu.harvard.iq.dataverse.authorization.DataverseRole; import edu.harvard.iq.dataverse.authorization.RoleAssignee; import edu.harvard.iq.dataverse.authorization.groups.GroupServiceBean; +import edu.harvard.iq.dataverse.authorization.providers.oauth2.oidc.OIDCAuthProvider; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.GuestUser; import edu.harvard.iq.dataverse.authorization.users.PrivateUrlUser; @@ -68,11 +70,13 @@ import java.net.URI; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Optional; import java.util.UUID; import java.util.concurrent.Callable; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; import javax.ejb.EJB; import javax.ejb.EJBException; import javax.json.Json; @@ -444,6 +448,8 @@ private AuthenticatedUser findAuthenticatedUserOrDie( String key, String wfid ) // TODO: Only usable for OIDC users for now, just look it up via the subject. // This will need to be modified to provide mappings somehow for existing non-OIDC-users. + // TODO: If we keep the current login infrastructure alive, we should introduce a common static + // method in OIDCAuthProvider to create the identifier in both places. AuthenticatedUser authUser = authSvc.getAuthenticatedUser(userInfo.getSubject().getValue()); // TODO: this is code dup par excellence. Needs refactoring. Maybe fine for Proof-of-Concept. @@ -499,7 +505,6 @@ Optional getOidcBearerToken(HttpServletRequest request) { * TODO: This is a proof of concept, providing value for IQSS#9229 and first steps for our SPA move. It ... * - will need more tweaks (see inline comments), * - should be extended to support JWT access tokens to avoid the extra detour to the OIDC provider, - * - will need a feature flag activation either here or in calling code, * - needs to be moved to a distinct place when we head for authentication filters in future iterations. *

* @@ -509,30 +514,50 @@ Optional getOidcBearerToken(HttpServletRequest request) { UserInfo verifyOidcBearerToken(String token) throws WrappedResponse { try { BearerAccessToken accessToken = BearerAccessToken.parse(token); + + // Get list of all authentication providers using Open ID Connect + List providers = authSvc.getAuthenticationProviderIdsOfType(OIDCAuthProvider.class).stream() + .map(providerId -> (OIDCAuthProvider) authSvc.getAuthenticationProvider(providerId)) + .collect(Collectors.toUnmodifiableList()); + + // Iterate over all OIDC providers if multiple. + for (OIDCAuthProvider provider : providers) { - // Retrieve data of the user accessing the API from the provider. - // No need to introspect the token here, the userInfoRequest also validates the token, as the provider - // is the source of truth. - // TODO: retrieve the userinfo endpoint from the OIDC provider already present via AuthenticationServiceBean - HTTPResponse response = new UserInfoRequest(URI.create("http://localhost:8090/realms/master/protocol/openid-connect/userinfo"), accessToken) - .toHTTPRequest() - .send(); - - UserInfoResponse infoResponse = UserInfoResponse.parse(response); + // Retrieve data of the user accessing the API from the provider. + // No need to introspect the token here, the userInfoRequest also validates the token, as the provider + // is the source of truth. + try { + HTTPResponse response = new UserInfoRequest(provider.getUserInfoEndpointURI(), accessToken) + .toHTTPRequest() + .send(); - // If error, throw 401 error exception - if (! infoResponse.indicatesSuccess() ) { - ErrorObject error = infoResponse.toErrorResponse().getErrorObject(); - // TODO: make the response more explaining? - throw new WrappedResponse(error(Status.UNAUTHORIZED, "Could not retrieve user info from provider")); - } + UserInfoResponse infoResponse = UserInfoResponse.parse(response); - // Success --> return info - return infoResponse.toSuccessResponse().getUserInfo(); - } catch (ParseException | IOException e) { - // TODO: make the response more explaining - throw new WrappedResponse(error(Status.UNAUTHORIZED, "Could not retrieve user info from provider")); + // If error, throw 401 error exception + if (! infoResponse.indicatesSuccess() ) { + ErrorObject error = infoResponse.toErrorResponse().getErrorObject(); + logger.log(Level.FINE, + "UserInfo could not be retrieved by access token from provider {0}: {1}", + new String[]{provider.getId(), error.getDescription()}); + // Success, simply return the user info + } else { + return infoResponse.toSuccessResponse().getUserInfo(); + } + } catch (ParseException | IOException e) { + logger.log(Level.WARNING, + "Could not retrieve user info for provider " + provider.getId() + ", skipping", e); + } + } + } catch (ParseException e) { + logger.log(Level.FINE, "Could not parse bearer access token", e); + throw new WrappedResponse(error(Status.UNAUTHORIZED, "Could not parse bearer access token")); } + + // No UserInfo returned means we have an invalid access token. (It could also mean we have no OIDC + // provider, but this would also mean this is an invalid request, as there will be no user available...) + // TODO: Should this include more details about the request? + logger.log(Level.FINE, "Unauthorized bearer access token detected"); + throw new WrappedResponse(error(Status.UNAUTHORIZED, "Unauthorized bearer access token")); } protected Dataverse findDataverseOrDie( String dvIdtf ) throws WrappedResponse { From b1a7b8580ba87fe8b6f941fd233f2de720825497 Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Thu, 15 Dec 2022 18:32:00 +0100 Subject: [PATCH 09/12] fix(api): don't default to guest user if OIDC bearer token is present --- src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index e9bd06323ca..3db0c846091 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -380,7 +380,7 @@ protected AuthenticatedUser findUserByApiToken( String apiKey ) { protected User findUserOrDie() throws WrappedResponse { final String requestApiKey = getRequestApiKey(); final String requestWFKey = getRequestWorkflowInvocationID(); - if (requestApiKey == null && requestWFKey == null && getRequestParameter(UrlSignerUtil.SIGNED_URL_TOKEN)==null) { + if (requestApiKey == null && requestWFKey == null && getRequestParameter(UrlSignerUtil.SIGNED_URL_TOKEN)==null && !(FeatureFlags.API_OIDC_ACCESS.enabled() && getOidcBearerToken(httpRequest).isPresent())) { return GuestUser.get(); } PrivateUrlUser privateUrlUser = privateUrlSvc.getPrivateUrlUserFromToken(requestApiKey); From 065630fe66b8e34435f5869866ab74b521eb93d3 Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Thu, 15 Dec 2022 18:35:48 +0100 Subject: [PATCH 10/12] fix(api): don't strip OIDC auth scheme from Authorization header Otherwise, BearerAccessToken.parse throws a ParseException since it expects the full HTTP Authorization header value. --- src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 3db0c846091..4606facc7f9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -492,7 +492,7 @@ Optional getOidcBearerToken(HttpServletRequest request) { String authHeader = request.getHeader(HttpHeaders.AUTHORIZATION); if (authHeader != null && authHeader.toLowerCase().startsWith(OIDC_AUTH_SCHEME.toLowerCase() + " ")) { - return Optional.of(authHeader.substring(OIDC_AUTH_SCHEME.length()+1)); + return Optional.of(authHeader); } else { return Optional.empty(); } From 6561bac99df9cdb7347aeb3dcb853dbc27d7b585 Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Thu, 15 Dec 2022 18:37:44 +0100 Subject: [PATCH 11/12] fix(api): send appropriate error response when OIDC user is unknown --- .../java/edu/harvard/iq/dataverse/api/AbstractApiBean.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 4606facc7f9..fcb7fcdeb02 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -456,6 +456,8 @@ private AuthenticatedUser findAuthenticatedUserOrDie( String key, String wfid ) if (authUser != null) { authUser = userSvc.updateLastApiUseTime(authUser); return authUser; + } else { + throw new WrappedResponse(badOidcUser(userInfo.getSubject().getValue())); } } //Just send info about the apiKey - workflow users will learn about invocationId elsewhere @@ -974,6 +976,10 @@ protected Response badWFKey( String wfId ) { String message = (wfId != null ) ? "Bad workflow invocationId " : "Please provide an invocationId query parameter (?invocationId=XXX) or via the HTTP header " + DATAVERSE_WORKFLOW_INVOCATION_HEADER_NAME; return error(Status.UNAUTHORIZED, message ); } + + protected Response badOidcUser(String oicdUserId ) { + return error(Status.UNAUTHORIZED, "OIDC user with identifier " + oicdUserId + " is unknown"); + } protected Response permissionError( PermissionException pe ) { return permissionError( pe.getMessage() ); From 62ce097abdfa8ea57edb994d3694560ba9b9513a Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Thu, 15 Dec 2022 18:39:05 +0100 Subject: [PATCH 12/12] fix(api): allow Authorization header in incoming API requests --- .../java/edu/harvard/iq/dataverse/api/ApiBlockingFilter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/ApiBlockingFilter.java b/src/main/java/edu/harvard/iq/dataverse/api/ApiBlockingFilter.java index 6bf852d25f7..d6941469366 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/ApiBlockingFilter.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/ApiBlockingFilter.java @@ -163,7 +163,7 @@ public void doFilter(ServletRequest sr, ServletResponse sr1, FilterChain fc) thr if (settingsSvc.isTrueForKey(SettingsServiceBean.Key.AllowCors, true )) { ((HttpServletResponse) sr1).addHeader("Access-Control-Allow-Origin", "*"); ((HttpServletResponse) sr1).addHeader("Access-Control-Allow-Methods", "PUT, GET, POST, DELETE, OPTIONS"); - ((HttpServletResponse) sr1).addHeader("Access-Control-Allow-Headers", "Accept, Content-Type, X-Dataverse-Key, Range"); + ((HttpServletResponse) sr1).addHeader("Access-Control-Allow-Headers", "Accept, Content-Type, X-Dataverse-Key, Range, Authorization"); ((HttpServletResponse) sr1).addHeader("Access-Control-Expose-Headers", "Accept-Ranges, Content-Range, Content-Encoding"); } fc.doFilter(sr, sr1);