Skip to content
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

#4940 - OAuth and SAML logins do not work if a servlet path is used #4944

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions inception/inception-project/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,12 @@
<artifactId>spring-security-core</artifactId>
</dependency>

<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
<scope>provided</scope>
</dependency>

<!-- Hibernate dependencies -->
<dependency>
<groupId>javax.persistence</groupId>
Expand Down Expand Up @@ -132,23 +138,15 @@
<scope>test</scope>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<executions>
<execution>
<id>default</id>
<phase>verify</phase>
<goals>
<goal>analyze-only</goal>
</goals>
</execution>
</executions>
<configuration>
<failOnWarning>true</failOnWarning>
<ignoredDependencies>
<ignoredDependency>javax.servlet:javax.servlet-api</ignoredDependency>
<ignoredDependency>org.hsqldb:hsqldb</ignoredDependency>
<ignoredDependency>org.springframework.boot:spring-boot-starter-data-jpa</ignoredDependency>
</ignoredDependencies>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1157,10 +1157,10 @@ public Realm getRealm(String aRealmId)
long projectId = Long.valueOf(substringAfter(aRealmId, REALM_PROJECT_PREFIX));
Project project = getProject(projectId);
if (project != null) {
return new Realm(aRealmId, project.getName());
return new Realm(aRealmId, "<Project> " + project.getName());
}
else {
return new Realm(aRealmId, "<Deleted project: " + projectId + ">");
return new Realm(aRealmId, "<Project (deleted)>: " + projectId + ">");
}
}
}
1 change: 1 addition & 0 deletions inception/inception-security/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@
<artifactId>spring-boot-autoconfigure</artifactId>
</dependency>
</dependencies>

<build>
<pluginManagement>
<plugins>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,7 @@ public Realm(String aId)
public Realm(String aId, String aName)
{
id = aId;

if (aName != null) {
name = aName;
}
else {
if (aId == null) {
name = "<LOCAL>";
}
else {
name = "<" + aId + ">";
}
}
name = aName;
}

public String getId()
Expand All @@ -68,7 +57,15 @@ public String getId()

public String getName()
{
return name;
if (name != null) {
return name;
}

if (id == null) {
return "<LOCAL>";
}

return "<" + id + ">";
}

@Override
Expand All @@ -89,15 +86,15 @@ public int hashCode()

public static int compareRealms(Realm aOne, Realm aOther)
{
if (aOne.getId() == null && aOther.getId() == null) {
if (aOne.id == null && aOther.id == null) {
return 0;
}

if (aOne.getId() == null) {
if (aOne.id == null) {
return -1;
}

if (aOther.getId() == null) {
if (aOther.id == null) {
return 1;
}

Expand All @@ -106,18 +103,18 @@ public static int compareRealms(Realm aOne, Realm aOther)

public static Realm forProject(long aProjectId, String aProjectName)
{
return new Realm(REALM_PROJECT_PREFIX + aProjectId, aProjectName);
return new Realm(REALM_PROJECT_PREFIX + aProjectId, "<Project>" + aProjectName);
}

public static Realm forExternalOAuth(ClientRegistration aClientRegistration)
{
return new Realm(REALM_EXTERNAL_PREFIX + aClientRegistration.getRegistrationId(),
aClientRegistration.getClientName());
"<External> " + aClientRegistration.getClientName());
}

public static Realm forExternalSaml(String aAuthenticationUri, String aRegistrationId)
{
return new Realm(REALM_EXTERNAL_PREFIX + aRegistrationId, aRegistrationId);
return new Realm(REALM_EXTERNAL_PREFIX + aRegistrationId, "<External> " + aRegistrationId);
}

public static Realm local()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import javax.persistence.EntityManager;
import javax.persistence.PersistenceContext;
import javax.servlet.ServletContext;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
Expand Down Expand Up @@ -132,11 +133,11 @@ public OAuth2Adapter oAuth2Adapter(@Lazy UserDao aUserRepository,
}

@Bean
public Saml2Adapter saml2Adapter(@Lazy UserDao aUserRepository,
public Saml2Adapter saml2Adapter(@Lazy ServletContext aContext, @Lazy UserDao aUserRepository,
@Lazy OverridableUserDetailsManager aUserDetailsManager,
@Lazy Optional<RelyingPartyRegistrationRepository> aRelyingPartyRegistrationRepository)
{
return new Saml2AdapterImpl(aUserRepository, aUserDetailsManager,
return new Saml2AdapterImpl(aContext, aUserRepository, aUserDetailsManager,
aRelyingPartyRegistrationRepository);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,15 +195,15 @@ private void denyAccessToUsersWithIllegalUsername(String aUsername)
String messages = userNameValidationResult.stream() //
.map(ValidationError::getMessage) //
.collect(joining("\n- ", "\n- ", ""));
LOG.info("Prevented login of user [{}] with illegal username: {}", aUsername, messages);
LOG.warn("Prevented login of user [{}] with illegal username: {}", aUsername, messages);
throw new BadCredentialsException("Illegal username");
}
}

private void denyAccessOfRealmsDoNotMatch(String aExpectedRealm, User aUser)
{
if (!aExpectedRealm.equals(aUser.getRealm())) {
LOG.info("Prevented login of user {} from realm [{}] via realm [{}]", aUser,
LOG.warn("Prevented login of user {} from realm [{}] via realm [{}]", aUser,
aUser.getRealm(), aExpectedRealm);
throw new BadCredentialsException("Realm mismatch");
}
Expand All @@ -212,7 +212,7 @@ private void denyAccessOfRealmsDoNotMatch(String aExpectedRealm, User aUser)
private void denyAccessToDeactivatedUsers(User aUser)
{
if (!aUser.isEnabled()) {
LOG.info("Prevented login of locally deactivated user {}", aUser);
LOG.warn("Prevented login of locally deactivated user {}", aUser);
throw new DisabledException("User deactivated");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,19 @@
package de.tudarmstadt.ukp.inception.security.saml;

import static de.tudarmstadt.ukp.clarin.webanno.security.UserDao.REALM_EXTERNAL_PREFIX;
import static java.util.Collections.emptyMap;
import static java.util.stream.Collectors.joining;
import static org.apache.commons.lang3.StringUtils.removeEnd;

import java.lang.invoke.MethodHandles;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import javax.servlet.ServletContext;

import org.apache.wicket.validation.ValidationError;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -52,16 +55,18 @@ public class Saml2AdapterImpl
{
private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

private static final String authenticationRequestUri = "/saml2/authenticate/";
private static final String AUTHENTICATION_REQUEST_URI = "/saml2/authenticate/";

private final ServletContext context;
private final UserDao userRepository;
private final OverridableUserDetailsManager userDetailsManager;
private final Optional<RelyingPartyRegistrationRepository> relyingPartyRegistrationRepository;

public Saml2AdapterImpl(@Lazy UserDao aUserRepository,
public Saml2AdapterImpl(@Lazy ServletContext aContext, @Lazy UserDao aUserRepository,
@Lazy OverridableUserDetailsManager aUserDetailsManager,
@Lazy Optional<RelyingPartyRegistrationRepository> aRelyingPartyRegistrationRepository)
{
context = aContext;
userRepository = aUserRepository;
userDetailsManager = aUserDetailsManager;
relyingPartyRegistrationRepository = aRelyingPartyRegistrationRepository;
Expand All @@ -77,15 +82,18 @@ public Saml2AdapterImpl(@Lazy UserDao aUserRepository,
public Map<String, String> getSamlRelyingPartyRegistrations()
{
if (relyingPartyRegistrationRepository.isEmpty()) {
return Collections.emptyMap();
return emptyMap();
}

Map<String, String> idps = new LinkedHashMap<>();
if (relyingPartyRegistrationRepository.get() instanceof Iterable) {
Iterable<RelyingPartyRegistration> repo = (Iterable<RelyingPartyRegistration>) //
relyingPartyRegistrationRepository.get();
repo.forEach((p) -> idps.put(authenticationRequestUri + p.getRegistrationId(),
p.getRegistrationId()));
var idps = new LinkedHashMap<String, String>();

if (relyingPartyRegistrationRepository.get() instanceof Iterable repo) {
var base = removeEnd(context.getContextPath(), "/") + AUTHENTICATION_REQUEST_URI;
repo.forEach(it -> {
if (it instanceof RelyingPartyRegistration p) {
idps.put(base + p.getRegistrationId(), p.getRegistrationId());
}
});
}

return idps;
Expand Down Expand Up @@ -160,15 +168,15 @@ private void denyAccessToUsersWithIllegalUsername(String aUsername)
String messages = userNameValidationResult.stream() //
.map(ValidationError::getMessage) //
.collect(joining("\n- ", "\n- ", ""));
LOG.info("Prevented login of user [{}] with illegal username: {}", aUsername, messages);
LOG.warn("Prevented login of user [{}] with illegal username: {}", aUsername, messages);
throw new BadCredentialsException("Illegal username");
}
}

private void denyAccessOfRealmsDoNotMatch(String aExpectedRealm, User aUser)
{
if (!aExpectedRealm.equals(aUser.getRealm())) {
LOG.info("Prevented login of user {} from realm [{}] via realm [{}]", aUser,
LOG.warn("Prevented login of user {} from realm [{}] via realm [{}]", aUser,
aUser.getRealm(), aExpectedRealm);
throw new BadCredentialsException("Realm mismatch");
}
Expand All @@ -177,7 +185,7 @@ private void denyAccessOfRealmsDoNotMatch(String aExpectedRealm, User aUser)
private void denyAccessToDeactivatedUsers(User aUser)
{
if (!aUser.isEnabled()) {
LOG.info("Prevented login of locally deactivated user {}", aUser);
LOG.warn("Prevented login of locally deactivated user {}", aUser);
throw new DisabledException("User deactivated");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@
{product-name} can authenticate a user against a OAuth2/OIDC-compatible identity provider. OAuth2/OIDC providers can be configured alongside the usual form-based login and SAML2.
It is **not** compatible with the <<sect_security_authentication_preauth,external pre-authentication>> and does not require setting the `auth.mode` property.

The following example configuration declares an OAuth2 service connection named `inception-client`
The following example configuration declares an OAuth2 service connection named `inception-client-oauth`
which uses a Keycloak instance configured for OAuth2 running at
`http://localhost:8180/realms/inception-demo`. The OAuth2 support of {product-name} should work with
any OAuth2/OIDC-compatible identity provider. For more details, please
refer to the link:https://docs.spring.io/spring-security/reference/servlet/oauth2/client/authorization-grants.html[Spring Security OAuth2 documentation].

.Example: Authenticate against a local Keycloak
----
spring.security.oauth2.client.registration.inception-client.client-name=Keycloak
spring.security.oauth2.client.registration.inception-client.client-id=inception-client
spring.security.oauth2.client.registration.inception-client.client-secret=ENCRYPTED_CLIENT_SECRET
spring.security.oauth2.client.registration.inception-client.scope=openid, profile
spring.security.oauth2.client.registration.inception-client.authorization-grant-type=authorization_code
spring.security.oauth2.client.registration.inception-client.redirect-uri=http://localhost:8080/login/oauth2/code/inception-client
spring.security.oauth2.client.provider.inception-client.issuer-uri=http://localhost:8180/realms/inception-demo
spring.security.oauth2.client.provider.inception-client.user-name-attribute=preferred_username
spring.security.oauth2.client.registration.inception-client-oauth.client-name=Keycloak
spring.security.oauth2.client.registration.inception-client-oauth.client-id=inception-client-oauth
spring.security.oauth2.client.registration.inception-client-oauth.client-secret=ENCRYPTED_CLIENT_SECRET
spring.security.oauth2.client.registration.inception-client-oauth.scope=openid, profile
spring.security.oauth2.client.registration.inception-client-oauth.authorization-grant-type=authorization_code
spring.security.oauth2.client.registration.inception-client-oauth.redirect-uri=http://localhost:8080/login/oauth2/code/inception-client-oauth
spring.security.oauth2.client.provider.inception-client-oauth.issuer-uri=http://localhost:8180/realms/inception-demo
spring.security.oauth2.client.provider.inception-client-oauth.user-name-attribute=preferred_username
----

NOTE: The following instructions run Keycloak in development mode. This is **not** meant for
Expand All @@ -47,11 +47,11 @@ If you want to try this with a local testing instance of Keycloak, you can do th
* Download link:https://www.keycloak.org[Keycloak]
* Run it using `./kc.sh start-dev --http-port 8180`
* Configure a new realm called `inception-demo`
* Define a new client `inception-client` and set the *Valid redirection URI* to `http://localhost:8080/login/oauth2/code/inception-client`.
* Define a new client `inception-client-oauth` and set the *Valid redirection URI* to `http://localhost:8080/login/oauth2/code/inception-client-oauth`.
* Replace the `ENCRYPTED_CLIENT_SECRET` in the example configuration above with the client secret from
the *Credentials* tab of the client in Keycloak.
* Add a new user in the *Manage users* area in Keycloak.

When you restart {product-name} and access the login page now, it should offer a login option called
*Keycloak*. You can change the label of that option by changing the
`security.oauth2.client.registration.inception-client.client-name` setting.
`security.oauth2.client.registration.inception-client-oauth.client-name` setting.
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@ providers can be configured alongside the usual form-based login and OAuth2.
It is **not** compatible with the <<sect_security_authentication_preauth,external pre-authentication>>
and does not require setting the `auth.mode` property.

The following example configuration declares a SAML2 service connection named `inception-client`
The following example configuration declares a SAML2 service connection named `inception-client-saml`
which uses a Keycloak instance configured for SAML2 running at
`http://localhost:8180/realms/inception-demo`. The SAML support of {product-name} should work with
any SAML2-compatible identity provider. For more details, please
refer to the link:https://docs.spring.io/spring-security/reference/servlet/saml2/index.html[Spring Security SAML2 documentation].

.Example: Authenticate against a local Keycloak
----
spring.security.saml2.relyingparty.registration.inception-client.assertingparty.entity-id=http://localhost:8180/realms/inception-demo
spring.security.saml2.relyingparty.registration.inception-client.assertingparty.verification.credentials[0].certificate-location=file:/srv/inception/keycloak-saml-idp.crt
spring.security.saml2.relyingparty.registration.inception-client.assertingparty.singlesignon.url=http://localhost:8180/realms/inception-demo/protocol/saml
spring.security.saml2.relyingparty.registration.inception-client.assertingparty.singlesignon.sign-request=false
spring.security.saml2.relyingparty.registration.inception-client-saml.assertingparty.entity-id=http://localhost:8180/realms/inception-demo
spring.security.saml2.relyingparty.registration.inception-client-saml.assertingparty.verification.credentials[0].certificate-location=file:/srv/inception/keycloak-saml-idp.crt
spring.security.saml2.relyingparty.registration.inception-client-saml.assertingparty.singlesignon.url=http://localhost:8180/realms/inception-demo/protocol/saml
spring.security.saml2.relyingparty.registration.inception-client-saml.assertingparty.singlesignon.sign-request=false
----

NOTE: The following instructions run Keycloak in development mode. This is **not** meant for
Expand All @@ -46,7 +46,7 @@ If you want to try this with a local testing instance of Keycloak, you can do th
* Run it using `./kc.sh start-dev --http-port 8180`
* Configure a new realm called `inception-demo`
* Define a new client and in the client wizard set
** Client ID: `http://localhost:8080/saml2/service-provider-metadata/inception-client`
** Client ID: `http://localhost:8080/saml2/service-provider-metadata/inception-client-saml`
** Client protocol: `saml`
** Save the client
* Set *Valid redirection URI* to `http://localhost:8080/login/saml2/sso/*`.
Expand All @@ -69,7 +69,7 @@ Save this file at the location indicated by the `....verification.credentials.ce
(here `/srv/inception/keycloak-saml-idp.crt`).

When you restart {product-name} and access the login page now, it should offer a login option called
*inception-client*. The SAML authentication does not allow defining the provider name shown on the login page independently from the registration ID. The registration ID (here `inception-client`) is defined in between the `registration`
*inception-client-saml*. The SAML authentication does not allow defining the provider name shown on the login page independently from the registration ID. The registration ID (here `inception-client-saml`) is defined in between the `registration`
and `assertingparty` parts of the configuration keys.

== Client certificate (optional)
Expand All @@ -88,19 +88,19 @@ openssl req -x509 -newkey rsa:2048 -keyout /srv/inception/inception.key -out /sr
----
cat /srv/inception/inception-saml.key /srv/inception/inception-saml.crt > /srv/inception/inception-saml.pem
----
* Open the previously defined client in Keycloak (e.g. `http://localhost:8080/saml2/service-provider-metadata/inception-client`)
* Set *Client signature Required* to `Off` and save the settings
* Open the previously defined client in Keycloak (e.g. `http://localhost:8080/saml2/service-provider-metadata/inception-client-saml`)
* Set *Client signature Required* to `On` and save the settings
* Now a new tab *Keys* should appear at the top. Switch to it.
* Click on *Import* and select *PEM* as the format, then upload the file `/srv/inception/inception.pem`
* Enable request signing in the `settings.properties` file
+
----
spring.security.saml2.relyingparty.registration.inception-client.assertingparty.singlesignon.sign-request=true
spring.security.saml2.relyingparty.registration.inception-client-saml.assertingparty.singlesignon.sign-request=true
----
* Configure the certificates for {product-name} to sign its requests
+
----
spring.security.saml2.relyingparty.registration.inception-client.signing.credentials[0].private-key-location=file:/srv/inception/inception-saml.key
spring.security.saml2.relyingparty.registration.inception-client.signing.credentials[0].certificate-location=file:/srv/inception/inception-saml.crt
spring.security.saml2.relyingparty.registration.inception-client-saml.signing.credentials[0].private-key-location=file:/srv/inception/inception-saml.key
spring.security.saml2.relyingparty.registration.inception-client-saml.signing.credentials[0].certificate-location=file:/srv/inception/inception-saml.crt
----

Loading
Loading