vaulttec/sonar-auth-oidc

The oidc plugin cannot work with ldap at the same time

klboke opened this issue · 6 comments

Open LDAP and OIDC plug-in at the same time, the same user will generate two accounts in soanr. This is not expected. It can be compatible by setting the Provider_key of the OIDC plug-in. For example

@ServerSide
public class OidcIdentityProvider implements OAuth2IdentityProvider {

  private static final Logger LOGGER = Loggers.get(OidcIdentityProvider.class);
  public static final String KEY = "sonarqube";

  private final OidcConfiguration config;
  private final OidcClient client;
  private final UserIdentityFactory userIdentityFactory;

  public OidcIdentityProvider(OidcConfiguration config, OidcClient client, UserIdentityFactory userIdentityFactory) {
    this.config = config;
    this.client = client;
    this.userIdentityFactory = userIdentityFactory;
  }

  @Override
  public String getKey() {
    return KEY;
  }

  @Override
  public String getName() {
    return config.loginButtonText();
  }

  @Override
  public Display getDisplay() {
    return Display.builder().setIconPath(config.iconPath()).setBackgroundColor(config.backgroundColor()).build();
  }

  @Override
  public boolean isEnabled() {
    return config.isEnabled();
  }

  @Override
  public boolean allowsUsersToSignUp() {
    return config.allowUsersToSignUp();
  }

  @Override
  public void init(InitContext context) {
    LOGGER.trace("Starting authentication workflow");
    if (!isEnabled()) {
      throw new IllegalStateException("OpenID Connect authentication is disabled");
    }
    String state = context.generateCsrfState();
    AuthenticationRequest authenticationRequest = client.getAuthenticationRequest(context.getCallbackUrl(), state);
    LOGGER.trace("Redirecting to authentication endpoint");
    context.redirectTo(authenticationRequest.toURI().toString());
  }

  @Override
  public void callback(CallbackContext context) {
    LOGGER.trace("Handling authentication response");
    context.verifyCsrfState();
    AuthorizationCode authorizationCode = client.getAuthorizationCode(context.getRequest());
    UserInfo userInfo = client.getUserInfo(authorizationCode, context.getCallbackUrl());
    UserIdentity userIdentity = userIdentityFactory.create(userInfo);
    LOGGER.debug("Authenticating user '{}' with groups {}", userIdentity.getProviderLogin(), userIdentity.getGroups());
    context.authenticate(userIdentity);
    LOGGER.trace("Redirecting to requested page");
    context.redirectToRequestedPage();
  }

}

how about this idea, I can mention a pr.

I've no idea how changing the identity provider's unique key from oidcto sonarqube should help here.

My best guess here is: If both identity providers are creating the same user then for one of them allowsUsersToSignUp must be turned off.

Turning off allowsUsersToSignUp does not solve the problem. Soanrqube's logic to query users when logging in is as follows, here is the key:
https://github.com/SonarSource/sonarqube/blob/master/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserRegistrarImpl.java
image
In the getUser() method, the two sql querying users use the key of IdentityProvider as the query condition, and when ldap is used, the key of IdentityProvider is sonarqube

All of these identity providers are managing it's on set of users. So it make's no sense to set the identity provider's unique key to the same key already used by another identity provider. The GitHub provider uses github or the SAML provider uses saml as it's key for a reason.

This is true, but from the current code point of view, only from the sole typing hand of IdentityProvider, can two different IdentityProviders work well at the same time. Another way of thinking, does it mean that soanrqube's interface design can be considered?

I'm not keen on using the same key from one of SQ's internal identity providers because it will interfere with the users created by other providers. Otherwise this unique key wouldn't be part of SQ's identity provider SPI.

Btw. why do you use an external identity provider for users which are already imported in SQ via LDAP? Why not omitting the LDAP provider if every user is available via OIDC as well?

Some details regarding the identity provider key sonarqube can be found in the description of SONAR-13930:

it should contain sonarqube, which is the identity provider id used for local users and for LDAP

And the unique identity provider key is used in the login form button URL /sessions/init/{provider key} as mentioned in the OAuth2IdentityProvider.init()method.