Skip to content

Commit

Permalink
SLE-910: Remove username/password authentication (#768)
Browse files Browse the repository at this point in the history
To still work on existing username/password based connections, the background logic still exists.

But when having an old username/password based connection and editing it, there is only the option to input a token (and therefore switch to this method for authentication).
  • Loading branch information
thahnen authored Nov 25, 2024
1 parent 6bfd611 commit bc08ce2
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 286 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.sonar.orchestrator.locator.URLLocation;
import java.io.File;
import java.io.IOException;
import java.time.Instant;
import java.util.Map;
import org.eclipse.core.runtime.FileLocator;
import org.eclipse.core.runtime.Path;
Expand All @@ -50,17 +51,26 @@
import org.sonarqube.ws.client.WsClientFactories;
import org.sonarqube.ws.client.projects.CreateRequest;
import org.sonarqube.ws.client.settings.SetRequest;
import org.sonarqube.ws.client.usertokens.GenerateRequest;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;

/** Every test class targeting SonarQube derives from here */
public abstract class AbstractSonarQubeConnectedModeTest extends AbstractSonarLintTest {
private static final String TIMESTAMP = Long.toString(Instant.now().toEpochMilli());
private static final String TOKEN_NAME = "SLE-IT-" + TIMESTAMP;

protected static WsClient adminWsClient;
protected static String token;

/** Should be used on @BeforeClass implementation for orchestrators to share the logic */
public static void prepare(OrchestratorRule orchestrator) {
adminWsClient = newAdminWsClient(orchestrator.getServer());
token = adminWsClient.userTokens()
.generate(new GenerateRequest().setName(TOKEN_NAME))
.getToken();

adminWsClient.settings().set(new SetRequest().setKey("sonar.forceAuthentication").setValue("true"));

try {
Expand Down Expand Up @@ -140,13 +150,10 @@ protected static void createConnectionAndBindProject(OrchestratorRule orchestrat
serverUrlPage.setUrl(orchestrator.getServer().getUrl());
wizard.next();

var authenticationModePage = new ServerConnectionWizard.AuthenticationModePage(wizard);
authenticationModePage.selectUsernamePasswordMode();
wizard.next();

assertThat(wizard.isNextEnabled()).isFalse();
var authenticationPage = new ServerConnectionWizard.AuthenticationPage(wizard);
authenticationPage.setUsername(username);
authenticationPage.setPassword(password);
authenticationPage.setToken(token);
assertThat(wizard.isNextEnabled()).isTrue();
wizard.next();

// as login can take time, wait for the next page to appear
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,8 @@ public void test_open_in_ide_assist_manual_binding() throws IOException, Interru
assertThat(serverUrlPage.getUrl()).isEqualTo(orchestrator.getServer().getUrl());
wizard.next();

var authenticationModePage = new ServerConnectionWizard.AuthenticationModePage(wizard);
authenticationModePage.selectUsernamePasswordMode();
wizard.next();

var authenticationPage = new ServerConnectionWizard.AuthenticationPage(wizard);
authenticationPage.setUsername(Server.ADMIN_LOGIN);
authenticationPage.setPassword(Server.ADMIN_PASSWORD);
authenticationPage.setToken(token);
wizard.next();

// as login can take time, wait for the next page to appear
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,24 +150,15 @@ public void configureServerFromNewWizard() {
assertThat(wizard.isNextEnabled()).isTrue();
wizard.next();

var authenticationModePage = new ServerConnectionWizard.AuthenticationModePage(wizard);
authenticationModePage.selectUsernamePasswordMode();
wizard.next();

var authenticationPage = new ServerConnectionWizard.AuthenticationPage(wizard);
assertThat(wizard.isNextEnabled()).isFalse();
authenticationPage.setUsername(Server.ADMIN_LOGIN);
assertThat(wizard.isNextEnabled()).isFalse();
authenticationPage.setPassword("wrong");
authenticationPage.setToken("Goo");
assertThat(wizard.isNextEnabled()).isTrue();

// until we change the ITs with the removal of the username / password authentication we check here once
assertThat(authenticationPage.getDeprecationMessage()).isEqualTo(authenticationPage.DEPRECATION_MESSAGE);

wizard.next();
new WaitUntil(new DialogMessageIsExpected(wizard, "Authentication failed"));

authenticationPage.setPassword(Server.ADMIN_PASSWORD);
authenticationPage.setToken(token);
wizard.next();

// as login can take time, wait for the next page to appear
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,21 +75,7 @@ public String getUrl() {
}
}

public static class AuthenticationModePage extends WizardPage {

public AuthenticationModePage(ReferencedComposite referencedComposite) {
super(referencedComposite);
}

public void selectUsernamePasswordMode() {
new RadioButton(this, 1).click();
}
}

public static class AuthenticationPage extends WizardPage {
public final String DEPRECATION_MESSAGE = "Authentication via username and password is deprecated and will "
+ "be removed in the future. Please use a token instead.";

public AuthenticationPage(ReferencedComposite referencedComposite) {
super(referencedComposite);
}
Expand All @@ -98,14 +84,6 @@ public void setToken(String token) {
new DefaultText(this).setText(token);
}

public void setUsername(String adminLogin) {
new DefaultText(this).setText(adminLogin);
}

public void setPassword(String password) {
new DefaultText(this, 1).setText(password);
}

public String getDeprecationMessage() {
return new DefaultLabel(this, 2).getText();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,10 @@ public static String getUsername(ConnectionFacade facade) throws StorageExceptio
return getFromSecure(facade, USERNAME_ATTRIBUTE);
}

/**
* @deprecated as only token authentication is supported from now on and this is saved in the username attribute!
*/
@Deprecated(since = "10.10", forRemoval = true)
@Nullable
public static String getPassword(ConnectionFacade facade) throws StorageException {
return getFromSecure(facade, PASSWORD_ATTRIBUTE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import org.sonarlint.eclipse.core.internal.utils.JobUtils;
import org.sonarlint.eclipse.ui.internal.Messages;
import org.sonarlint.eclipse.ui.internal.binding.BindingsView;
import org.sonarlint.eclipse.ui.internal.binding.wizard.connection.ServerConnectionModel.AuthMethod;
import org.sonarlint.eclipse.ui.internal.binding.wizard.connection.ServerConnectionModel.ConnectionType;
import org.sonarlint.eclipse.ui.internal.util.wizard.SonarLintWizardDialog;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.connection.common.TransientSonarCloudConnectionDto;
Expand Down Expand Up @@ -193,7 +192,7 @@ protected Either<TransientSonarQubeConnectionDto, TransientSonarCloudConnectionD
}

protected Either<TokenDto, UsernamePasswordDto> modelToCredentialDto() {
return model.getAuthMethod() == AuthMethod.TOKEN
return model.getPassword() == null
? Either.<TokenDto, UsernamePasswordDto>forLeft(new TokenDto(model.getUsername()))
: Either.<TokenDto, UsernamePasswordDto>forRight(new UsernamePasswordDto(model.getUsername(), model.getPassword()));
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,21 @@ public enum ConnectionType {
SONARCLOUD, ONPREMISE
}

public enum AuthMethod {
TOKEN, PASSWORD
}

private final boolean edit;
private final boolean fromConnectionSuggestion;
private ConnectionType connectionType = ConnectionType.SONARCLOUD;
private AuthMethod authMethod = AuthMethod.TOKEN;
private String connectionId;
private String serverUrl = SonarLintUtils.getSonarCloudUrl();
private String organization;
private String username;

/**
* @deprecated as only token authentication is supported from now on and this is saved in the username field!
*/
@Deprecated(since = "10.10", forRemoval = true)
@Nullable
private String password;

private boolean notificationsSupported;
private boolean notificationsDisabled;

Expand Down Expand Up @@ -97,7 +99,6 @@ public ServerConnectionModel(ConnectionFacade connection) {
SonarLintLogger.get().error(ERROR_READING_SECURE_STORAGE, e);
MessageDialog.openError(Display.getCurrent().getActiveShell(), ERROR_READING_SECURE_STORAGE, "Unable to read password from secure storage: " + e.getMessage());
}
this.authMethod = StringUtils.isBlank(password) ? AuthMethod.TOKEN : AuthMethod.PASSWORD;
}
this.notificationsDisabled = connection.areNotificationsDisabled();
}
Expand All @@ -122,22 +123,9 @@ public void setConnectionType(ConnectionType type) {
setServerUrl(null);
} else {
setServerUrl(SonarLintUtils.getSonarCloudUrl());
setAuthMethod(AuthMethod.TOKEN);
}
}

public AuthMethod getAuthMethod() {
return authMethod;
}

public void setAuthMethod(AuthMethod authMethod) {
var old = this.authMethod;
this.authMethod = authMethod;
firePropertyChange(PROPERTY_AUTH_METHOD, old, this.authMethod);
setUsername(null);
setPassword(null);
}

public String getConnectionId() {
return connectionId;
}
Expand Down Expand Up @@ -255,7 +243,7 @@ public boolean getNotificationsDisabled() {
}

public Either<TokenDto, UsernamePasswordDto> getTransientRpcCrendentials() {
if (authMethod == AuthMethod.TOKEN) {
if (password == null) {
return Either.forLeft(new TokenDto(username));
} else {
return Either.forRight(new UsernamePasswordDto(username, password));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.sonarlint.eclipse.core.internal.engine.connected.ConnectionFacade;
import org.sonarlint.eclipse.core.internal.utils.JobUtils;
import org.sonarlint.eclipse.core.resource.ISonarLintProject;
import org.sonarlint.eclipse.ui.internal.binding.wizard.connection.ServerConnectionModel.AuthMethod;
import org.sonarlint.eclipse.ui.internal.binding.wizard.connection.ServerConnectionModel.ConnectionType;
import org.sonarlint.eclipse.ui.internal.binding.wizard.project.ProjectBindingWizard;
import org.sonarlint.eclipse.ui.internal.util.wizard.SonarLintWizardDialog;
Expand All @@ -48,8 +47,6 @@ public class ServerConnectionWizard extends AbstractConnectionWizard {

private final ConnectionTypeWizardPage connectionTypeWizardPage;
private final UrlWizardPage urlPage;
private final AuthMethodWizardPage authMethodPage;
private final UsernamePasswordWizardPage credentialsPage;
private final OrganizationWizardPage orgPage;
private final ConnectionIdWizardPage connectionIdPage;
private final NotificationsWizardPage notifPage;
Expand All @@ -63,8 +60,6 @@ private ServerConnectionWizard(String title, ServerConnectionModel model, Connec
this.editedServer = editedServer;
connectionTypeWizardPage = new ConnectionTypeWizardPage(model);
urlPage = new UrlWizardPage(model);
authMethodPage = new AuthMethodWizardPage(model);
credentialsPage = new UsernamePasswordWizardPage(model);
orgPage = new OrganizationWizardPage(model);
connectionIdPage = new ConnectionIdWizardPage(model);
notifPage = new NotificationsWizardPage(model);
Expand Down Expand Up @@ -111,7 +106,12 @@ public static WizardDialog createDialog(Shell parent, ConnectionFacade connectio
protected void actualHandlePageChanging(PageChangingEvent event) {
var currentPage = (WizardPage) event.getCurrentPage();
var advance = getNextPage(currentPage) == event.getTargetPage();
if (advance && !redirectedAfterNotificationCheck && (currentPage == credentialsPage || currentPage == tokenPage)) {
if (advance && !redirectedAfterNotificationCheck && currentPage == tokenPage) {
// When having a username/password based connection and editing it, there is only the option to switch to a token
// available. In this case, when a token was generated, we will remove the password and set the authentication
// method to "TOKEN" that is then used on the "AbstractConnectionWizard#testConnection(...)" already!
model.setPassword(null);

if (!testConnection(null)) {
event.doit = false;
return;
Expand Down Expand Up @@ -155,8 +155,6 @@ protected void actualAddPages() {
addPage(connectionIdPage);
}
addPage(urlPage);
addPage(authMethodPage);
addPage(credentialsPage);
addPage(tokenPage);
addPage(orgPage);
addPage(notifPage);
Expand All @@ -169,18 +167,15 @@ protected IWizardPage getActualNextPage(IWizardPage page) {
return firstPageAfterConnectionType();
}
if (page == urlPage) {
return authMethodPage;
}
if (page == authMethodPage) {
return model.getAuthMethod() == AuthMethod.PASSWORD ? credentialsPage : tokenPage;
return tokenPage;
}

// This comes from Connection suggestion, we don't need anything from here!
if (page == tokenPage && model.isFromConnectionSuggestion()) {
return null;
}

if (page == credentialsPage || page == tokenPage) {
if (page == tokenPage) {
if (model.getConnectionType() == ConnectionType.SONARCLOUD) {
return orgPage;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ private void createOpenSecurityPageButton(Composite container) {

private void openTokenCreationPage() {
try {
var job = new GenerateTokenJob(model.getServerUrl(), model.getConnectionType() == ConnectionType.SONARCLOUD);
var job = new GenerateTokenJob(model.getServerUrl());
getContainer().run(true, true, job);
var response = job.getResponse();
var token = response.getToken();
Expand All @@ -138,19 +138,17 @@ private void handleReceivedToken(String token) {
static final class GenerateTokenJob implements IRunnableWithProgress {

private final String serverUrl;
private final boolean isSonarCloud;
private HelpGenerateUserTokenResponse response;

public GenerateTokenJob(String serverUrl, boolean isSonarCloud) {
public GenerateTokenJob(String serverUrl) {
this.serverUrl = serverUrl;
this.isSonarCloud = isSonarCloud;
}

@Override
public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException {
monitor.beginTask("Token generation", IProgressMonitor.UNKNOWN);
try {
var params = new HelpGenerateUserTokenParams(serverUrl, isSonarCloud);
var params = new HelpGenerateUserTokenParams(serverUrl);
var future = SonarLintBackendService.get().getBackend().getConnectionService().helpGenerateUserToken(params);
this.response = JobUtils.waitForFutureInIRunnableWithProgress(monitor, future);
} finally {
Expand Down
Loading

0 comments on commit bc08ce2

Please sign in to comment.