Skip to content

Commit

Permalink
Treat azure devops redirect response as unauthorized error (#755)
Browse files Browse the repository at this point in the history
Handle the 302 redirect response in the azure devops api client and throw unauthorized exception, so the dashboard can handle it properly. Also add 401 unauthorized and 403 forbidden to the unauthorized case.
  • Loading branch information
vinokurig committed Jan 24, 2025
1 parent 1d471e6 commit 38d03e4
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2024 Red Hat, Inc.
* Copyright (c) 2012-2025 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
Expand All @@ -12,9 +12,12 @@
package org.eclipse.che.api.factory.server.azure.devops;

import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
import static java.net.HttpURLConnection.HTTP_MOVED_TEMP;
import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
import static java.net.HttpURLConnection.HTTP_NO_CONTENT;
import static java.net.HttpURLConnection.HTTP_OK;
import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED;
import static java.time.Duration.ofSeconds;
import static org.eclipse.che.api.factory.server.azure.devops.AzureDevOps.formatAuthorizationHeader;
import static org.eclipse.che.commons.lang.StringUtils.trimEnd;
Expand All @@ -40,6 +43,7 @@
import org.eclipse.che.api.factory.server.scm.exception.ScmBadRequestException;
import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException;
import org.eclipse.che.api.factory.server.scm.exception.ScmItemNotFoundException;
import org.eclipse.che.api.factory.server.scm.exception.ScmUnauthorizedException;
import org.eclipse.che.commons.lang.concurrent.LoggingUncaughtExceptionHandler;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -80,7 +84,8 @@ public AzureDevOpsApiClient(
* <p>https://learn.microsoft.com/en-us/rest/api/azure/devops/graph/users/get?view=azure-devops-rest-7.0&tabs=HTTP
*/
public AzureDevOpsUser getUserWithOAuthToken(String token)
throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException {
throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException,
ScmUnauthorizedException {
final String url =
String.format(
"%s/_apis/profile/profiles/me?api-version=%s",
Expand All @@ -94,7 +99,8 @@ public AzureDevOpsUser getUserWithOAuthToken(String token)
* organization.
*/
public AzureDevOpsUser getUserWithPAT(String pat, String organization)
throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException {
throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException,
ScmUnauthorizedException {
final String url =
String.format(
"%s/%s/_apis/profile/profiles/me?api-version=%s",
Expand All @@ -103,7 +109,8 @@ public AzureDevOpsUser getUserWithPAT(String pat, String organization)
}

private AzureDevOpsUser getUser(String url, String authorizationHeader)
throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException {
throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException,
ScmUnauthorizedException {
final HttpRequest userDataRequest =
HttpRequest.newBuilder(URI.create(url))
.headers("Authorization", authorizationHeader)
Expand All @@ -129,7 +136,8 @@ private <T> T executeRequest(
HttpClient httpClient,
HttpRequest request,
Function<HttpResponse<InputStream>, T> responseConverter)
throws ScmBadRequestException, ScmItemNotFoundException, ScmCommunicationException {
throws ScmBadRequestException, ScmItemNotFoundException, ScmCommunicationException,
ScmUnauthorizedException {
try {
HttpResponse<InputStream> response =
httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
Expand All @@ -145,6 +153,11 @@ private <T> T executeRequest(
throw new ScmBadRequestException(body);
case HTTP_NOT_FOUND:
throw new ScmItemNotFoundException(body);
case HTTP_UNAUTHORIZED:
case HTTP_FORBIDDEN:
// Azure DevOps tries to redirect to the login page if the user is not authorized.
case HTTP_MOVED_TEMP:
throw new ScmUnauthorizedException(body, "azure-devops", "v2", "");
default:
throw new ScmCommunicationException(
"Unexpected status code " + response.statusCode() + " " + response,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2024 Red Hat, Inc.
* Copyright (c) 2012-2025 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
Expand Down Expand Up @@ -163,7 +163,10 @@ public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
personalAccessToken.getToken(), personalAccessToken.getScmOrganization());
}
return Optional.of(personalAccessToken.getScmUserName().equals(user.getEmailAddress()));
} catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
} catch (ScmItemNotFoundException
| ScmCommunicationException
| ScmBadRequestException
| ScmUnauthorizedException e) {
return Optional.of(Boolean.FALSE);
}
}
Expand All @@ -184,7 +187,7 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
user = azureDevOpsApiClient.getUserWithPAT(params.getToken(), params.getOrganization());
}
return Optional.of(Pair.of(Boolean.TRUE, user.getEmailAddress()));
} catch (ScmItemNotFoundException | ScmBadRequestException e) {
} catch (ScmItemNotFoundException | ScmBadRequestException | ScmUnauthorizedException e) {
return Optional.empty();
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2024 Red Hat, Inc.
* Copyright (c) 2012-2025 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
Expand All @@ -22,6 +22,7 @@
import org.eclipse.che.api.factory.server.scm.exception.ScmBadRequestException;
import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException;
import org.eclipse.che.api.factory.server.scm.exception.ScmItemNotFoundException;
import org.eclipse.che.api.factory.server.scm.exception.ScmUnauthorizedException;

/**
* Azure DevOps user data fetcher.
Expand All @@ -48,15 +49,17 @@ public AzureDevOpsUserDataFetcher(

@Override
protected GitUserData fetchGitUserDataWithOAuthToken(String token)
throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException {
throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException,
ScmUnauthorizedException {
AzureDevOpsUser user = azureDevOpsApiClient.getUserWithOAuthToken(token);
return new GitUserData(user.getDisplayName(), user.getEmailAddress());
}

@Override
protected GitUserData fetchGitUserDataWithPersonalAccessToken(
PersonalAccessToken personalAccessToken)
throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException {
throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException,
ScmUnauthorizedException {
AzureDevOpsUser user =
azureDevOpsApiClient.getUserWithPAT(
personalAccessToken.getToken(), personalAccessToken.getScmOrganization());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2024 Red Hat, Inc.
* Copyright (c) 2012-2025 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
Expand All @@ -18,6 +18,7 @@
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
import static java.net.HttpURLConnection.HTTP_MOVED_TEMP;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
Expand Down Expand Up @@ -115,9 +116,22 @@ public void shouldThrowUnauthorizedExceptionIfTokenIsNotValid() throws Exception
when(oAuthAPI.getOrRefreshToken(anyString())).thenReturn(oAuthToken);
stubFor(
get(urlEqualTo("/_apis/profile/profiles/me?api-version=7.0"))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("token " + azureOauthToken))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("Bearer " + azureOauthToken))
.willReturn(aResponse().withStatus(HTTP_FORBIDDEN)));

personalAccessTokenFetcher.fetchPersonalAccessToken(subject, wireMockServer.url("/"));
}

@Test(expectedExceptions = ScmUnauthorizedException.class)
public void shouldThrowUnauthorizedExceptionOnRedirectResponse() throws Exception {
Subject subject = new SubjectImpl("Username", "id1", "token", false);
OAuthToken oAuthToken = newDto(OAuthToken.class).withToken(azureOauthToken).withScope("");
when(oAuthAPI.getOrRefreshToken(anyString())).thenReturn(oAuthToken);
stubFor(
get(urlEqualTo("/_apis/profile/profiles/me?api-version=7.0"))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("Bearer " + azureOauthToken))
.willReturn(aResponse().withStatus(HTTP_MOVED_TEMP)));

personalAccessTokenFetcher.fetchPersonalAccessToken(subject, wireMockServer.url("/"));
}
}

0 comments on commit 38d03e4

Please sign in to comment.