Back-port #3636 to 7.1

stroom-security/stroom-security-identity/src/main/java/stroom/security/identity/authenticate/AuthenticationResource.java

@@ -79,7 +79,8 @@ LoginResponse login(
             operationId = "logout")
     Boolean logout(
             @Context @NotNull HttpServletRequest request,
-            @QueryParam(OpenId.REDIRECT_URI) @NotNull String redirectUri);
+            @QueryParam(OpenId.POST_LOGOUT_REDIRECT_URI) @NotNull String postLogoutRedirectUri,
+            @QueryParam(OpenId.STATE) @NotNull String state);
 
     @POST
     @Path("/noauth/confirmPassword")

stroom-security/stroom-security-identity/src/main/java/stroom/security/identity/authenticate/AuthenticationResourceImpl.java

@@ -128,9 +128,9 @@ public LoginResponse login(final HttpServletRequest request,
      */
     @Timed
     @Override
-    public Boolean logout(
-            final HttpServletRequest request,
-            final String redirectUri) {
+    public Boolean logout(final HttpServletRequest request,
+                          final String postLogoutRedirectUri,
+                          final String state) {
         LOGGER.debug("Received a logout request");
         final AuthenticateEventAction.Builder<Void> eventBuilder = event.logging.AuthenticateEventAction.builder()
                 .withLogonType(AuthenticateLogonType.INTERACTIVE)
@@ -156,13 +156,9 @@ public Boolean logout(
         }
 
         try {
-            UriBuilder uriBuilder = UriBuilder.fromUri(redirectUri);
-            final String promptParam = UrlUtils.getLastParam(request, OpenId.PROMPT);
-            if (!Strings.isNullOrEmpty(promptParam)) {
-                uriBuilder = UriBuilderUtil.addParam(uriBuilder, OpenId.PROMPT, promptParam);
-            }
-
-            throw new RedirectionException(Status.SEE_OTHER, uriBuilder.build());
+            UriBuilder uriBuilder = UriBuilder.fromUri(postLogoutRedirectUri);
+            uriBuilder = UriBuilderUtil.addParam(uriBuilder, OpenId.STATE, state);
+            throw new RedirectionException(Status.TEMPORARY_REDIRECT, uriBuilder.build());
         } finally {
             stroomEventLoggingServiceProvider.get().log(
                     "AuthenticationResourceImpl.Logout",

stroom-security/stroom-security-identity/src/main/java/stroom/security/identity/exceptions/mappers/NoCertificateExceptionMapper.java

@@ -50,6 +50,6 @@ class NoCertificateExceptionMapper implements ExceptionMapper<NoCertificateExcep
     @Override
     public Response toResponse(NoCertificateException exception) {
         LOGGER.debug("Unable to create a token for this user. Redirecting to login as a backup method.", exception);
-        return Response.seeOther(uriFactory.uiUri(AuthenticationService.SIGN_IN_URL_PATH)).build();
+        return Response.temporaryRedirect(uriFactory.uiUri(AuthenticationService.SIGN_IN_URL_PATH)).build();
     }
 }

stroom-security/stroom-security-identity/src/main/java/stroom/security/identity/openid/OpenIdResourceImpl.java

@@ -124,7 +124,7 @@ public void auth(final HttpServletRequest request,
                     eventBuilder.build());
         }
 
-        throw new RedirectionException(Status.SEE_OTHER, result.getUri());
+        throw new RedirectionException(Status.TEMPORARY_REDIRECT, result.getUri());
     }
 
     @Timed

stroom-security/stroom-security-identity/src/main/java/stroom/security/identity/openid/OpenIdService.java

@@ -1,5 +1,6 @@
 package stroom.security.identity.openid;
 
+import stroom.config.common.UriFactory;
 import stroom.security.identity.authenticate.api.AuthenticationService;
 import stroom.security.identity.authenticate.api.AuthenticationService.AuthState;
 import stroom.security.identity.authenticate.api.AuthenticationService.AuthStatus;
@@ -44,20 +45,23 @@ class OpenIdService {
     private final AuthenticationService authenticationService;
     private final OpenIdClientFactory openIdClientDetailsFactory;
     private final IdentityConfig identityConfig;
+    private final UriFactory uriFactory;
 
     @Inject
     OpenIdService(final AccessCodeCache accessCodeCache,
                   final RefreshTokenCache refreshTokenCache,
                   final TokenBuilderFactory tokenBuilderFactory,
                   final AuthenticationService authenticationService,
                   final OpenIdClientFactory openIdClientDetailsFactory,
-                  final IdentityConfig identityConfig) {
+                  final IdentityConfig identityConfig,
+                  final UriFactory uriFactory) {
         this.accessCodeCache = accessCodeCache;
         this.refreshTokenCache = refreshTokenCache;
         this.tokenBuilderFactory = tokenBuilderFactory;
         this.authenticationService = authenticationService;
         this.openIdClientDetailsFactory = openIdClientDetailsFactory;
         this.identityConfig = identityConfig;
+        this.uriFactory = uriFactory;
     }
 
     public AuthResult auth(final HttpServletRequest request,
@@ -72,6 +76,8 @@ public AuthResult auth(final HttpServletRequest request,
         AuthStatus authStatus = null;
 
         OpenIdClient oAuth2Client = openIdClientDetailsFactory.getClient(clientId);
+        // After sign in attempts we want to come back here.
+        final String postSignInRedirectUri = getPostSignInRedirectUri(request);
 
         final Pattern pattern = Pattern.compile(oAuth2Client.getUriPattern());
         if (!pattern.matcher(redirectUri).matches()) {
@@ -94,18 +100,18 @@ public boolean isNew() {
                 }
             };
 
-            result = authenticationService.createSignInUri(redirectUri);
+            result = authenticationService.createSignInUri(postSignInRedirectUri);
 
         } else {
             // If the prompt is 'login' then we always want to prompt the user to login in with username and password.
-            final boolean requireLoginPrompt = prompt != null && prompt.equalsIgnoreCase("login");
+            final boolean requireLoginPrompt = prompt != null && prompt.equalsIgnoreCase(OpenId.LOGIN_PROMPT);
             if (requireLoginPrompt) {
                 LOGGER.info("Relying party requested a user login page by using 'prompt=login'");
             }
 
             if (requireLoginPrompt) {
                 LOGGER.debug("Login has been requested by the RP");
-                result = authenticationService.createSignInUri(redirectUri);
+                result = authenticationService.createSignInUri(postSignInRedirectUri);
 
             } else {
                 // We need to make sure our understanding of the session is correct
@@ -118,15 +124,15 @@ public boolean isNew() {
                             authStatus.getError().get().getReason().value(),
                             authStatus.getError().get().getMessage());
                     //Send back to log in with username/password
-                    result = authenticationService.createSignInUri(redirectUri);
+                    result = authenticationService.createSignInUri(postSignInRedirectUri);
 
                 } else if (authStatus.getAuthState().isPresent()) {
                     // If we have an authenticated session then the user is logged in
                     final AuthState authState = authStatus.getAuthState().get();
 
-                    // If the users password still needs tp be changed then send them back to the login page.
+                    // If the users password still needs to be changed then send them back to the login page.
                     if (authState.isRequirePasswordChange()) {
-                        result = authenticationService.createSignInUri(redirectUri);
+                        result = authenticationService.createSignInUri(postSignInRedirectUri);
 
                     } else {
                         LOGGER.debug("User has a session, sending them back to the RP");
@@ -150,7 +156,7 @@ public boolean isNew() {
 
                 } else {
                     LOGGER.debug("User has no session and no certificate - sending them to login.");
-                    result = authenticationService.createSignInUri(redirectUri);
+                    result = authenticationService.createSignInUri(postSignInRedirectUri);
                 }
 
             }
@@ -269,6 +275,24 @@ private URI buildRedirectionUrl(String redirectUri, String code, String state) {
                 .build();
     }
 
+    private String getPostSignInRedirectUri(final HttpServletRequest request) {
+        // We have a new request so we're going to redirect with an AuthenticationRequest.
+        // Get the redirect URL for the auth service from the current request.
+        final String originalPath = request.getRequestURI() + Optional.ofNullable(request.getQueryString())
+                .map(queryStr -> "?" + queryStr)
+                .orElse("");
+
+        // Dropwiz is likely sat behind Nginx with requests reverse proxied to it
+        // so we need to append just the path/query part to the public URI defined in config
+        // rather than using the full url of the request
+        final String uri = uriFactory.publicUri(originalPath).toString();
+
+        final UriBuilder uriBuilder = UriBuilder.fromUri(uri);
+        // Ensure we have no prompt so we don't go round in circles.
+        uriBuilder.replaceQueryParam(OpenId.PROMPT, "");
+        return uriBuilder.build().toString();
+    }
+
     private TokenResponse createTokenResponse(final String clientId,
                                               final String subject,
                                               final String nonce,

stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/AuthenticationState.java

@@ -1,14 +1,46 @@
 package stroom.security.impl;
 
+import stroom.security.openid.api.OpenId;
+
+import java.util.Set;
+import javax.ws.rs.core.UriBuilder;
+
 public class AuthenticationState {
+
+    private static final Set<String> RESERVED_PARAMS = Set.of(
+            OpenId.AUTH_USER,
+            OpenId.CLIENT_ID,
+            OpenId.CLIENT_SECRET,
+            OpenId.CODE,
+            OpenId.GRANT_TYPE,
+            OpenId.NONCE,
+            OpenId.PROMPT,
+            OpenId.REDIRECT_URI,
+            OpenId.RESPONSE_TYPE,
+            OpenId.SCOPE,
+            OpenId.STATE
+    );
+
     private final String id;
-    private final String uri;
+    private final String url;
+    private final String initiatingUri;
+    private final String redirectUri;
     private final String nonce;
+    private final boolean prompt;
 
-    AuthenticationState(final String id, final String uri, final String nonce) {
+    AuthenticationState(final String id,
+                        final String url,
+                        final String nonce,
+                        final boolean prompt) {
         this.id = id;
-        this.uri = uri;
+        this.url = url;
         this.nonce = nonce;
+        this.prompt = prompt;
+
+        // Make sure the initiating URI doesn't contain any reserved OIDC params.
+        this.initiatingUri = createInitiatingUri(url);
+        // Create a simple redirect URI.
+        this.redirectUri = createRedirectUri(url);
     }
 
     /**
@@ -21,12 +53,21 @@ public String getId() {
     }
 
     /**
-     * The URI of the originating request that this state is linked to.
+     * The URI of the initiating request that this state is linked to.
      *
-     * @return The URL of the originating request that this state is linked to.
+     * @return The URL of the initiating request that this state is linked to.
      */
-    public String getUri() {
-        return uri;
+    public String getInitiatingUri() {
+        return initiatingUri;
+    }
+
+    /**
+     * The URI that should be sent to the IDP that the IDP will use to redirect back one authenticated.
+     *
+     * @return The URI that should be sent to the IDP that the IDP will use to redirect back one authenticated.
+     */
+    public String getRedirectUri() {
+        return redirectUri;
     }
 
     /**
@@ -44,12 +85,50 @@ public String getNonce() {
         return nonce;
     }
 
+    /**
+     * Determine if the next auth call should force a prompt.
+     *
+     * @return True if the next auth call should force a prompt.
+     */
+    public boolean isPrompt() {
+        return prompt;
+    }
+
     @Override
     public String toString() {
         return "AuthenticationState{" +
                 "id='" + id + '\'' +
-                ", uri='" + uri + '\'' +
+                ", url='" + url + '\'' +
+                ", initiatingUri='" + initiatingUri + '\'' +
+                ", redirectUri='" + redirectUri + '\'' +
                 ", nonce='" + nonce + '\'' +
+                ", prompt=" + prompt +
                 '}';
     }
+
+    private static String createInitiatingUri(final String url) {
+        final UriBuilder uriBuilder = UriBuilder.fromUri(url);
+
+        // When the auth service has performed authentication it will redirect
+        // back to the current URL with some additional parameters (e.g.
+        // `state` and `accessCode`). It is important that these parameters are
+        // not provided by our redirect URL else the redirect URL that the
+        // authentication service redirects back to may end up with multiple
+        // copies of these parameters which will confuse Stroom as it will not
+        // know which one of the param values to use (i.e. which were on the
+        // original redirect request and which have been added by the
+        // authentication service). For this reason we will cleanse the URL of
+        // any reserved parameters here. The authentication service should do
+        // the same to the redirect URL before adding its additional
+        // parameters.
+        RESERVED_PARAMS.forEach(param -> uriBuilder.replaceQueryParam(param, new Object[0]));
+
+        return uriBuilder.build().toString();
+    }
+
+    private static String createRedirectUri(final String url) {
+        final UriBuilder uriBuilder = UriBuilder.fromUri(url);
+        uriBuilder.replaceQuery("");
+        return uriBuilder.build().toString();
+    }
 }

stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/AuthenticationStateSessionUtil.java

@@ -31,7 +31,9 @@ private AuthenticationStateSessionUtil() {
      * that Stroom makes to the Authentication Service. When Stroom is subsequently called the state is provided in the
      * URL to allow verification that the return request was expected.
      */
-    public static AuthenticationState create(final HttpServletRequest request, final String url) {
+    public static AuthenticationState create(final HttpServletRequest request,
+                                             final String url,
+                                             final boolean prompt) {
         final String stateId = createRandomString(8);
         final String nonce = createRandomString(20);
 
@@ -45,7 +47,7 @@ public static AuthenticationState create(final HttpServletRequest request, final
                     request.getRequestURI());
         });
 
-        final AuthenticationState state = new AuthenticationState(stateId, url, nonce);
+        final AuthenticationState state = new AuthenticationState(stateId, url, nonce, prompt);
 
         final Cache<String, AuthenticationState> cache = getOrCreateCache(request);
         cache.put(stateId, state);
@@ -79,7 +81,7 @@ private static Cache<String, AuthenticationState> getOrCreateCache(final HttpSer
                     LOGGER.debug("Creating cache for session {}", sessionId);
                     cache = Caffeine.newBuilder()
                             .maximumSize(100)
-                            .expireAfterWrite(1, TimeUnit.MINUTES)
+                            .expireAfterWrite(10, TimeUnit.MINUTES)
                             .removalListener((key, value, cause) ->
                                     LOGGER.debug(() -> LogUtil.message(
                                             "Removing entry: {}, cause: {}, session: {}",