fixup! Fix gitlab oauth token validation request

eclipse-che · Apr 4, 2024 · c51919d · c51919d
1 parent 1fc4e07
commit c51919d
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 6 deletions.
diff --git a/wsmaster/che-core-api-auth-gitlab/pom.xml b/wsmaster/che-core-api-auth-gitlab/pom.xml
@@ -35,6 +35,10 @@
             <groupId>com.google.inject</groupId>
             <artifactId>guice</artifactId>
         </dependency>
+        <dependency>
+            <groupId>com.google.oauth-client</groupId>
+            <artifactId>google-oauth-client</artifactId>
+        </dependency>
         <dependency>
             <groupId>jakarta.inject</groupId>
             <artifactId>jakarta.inject-api</artifactId>

diff --git a/...pi-auth-gitlab/src/main/java/org/eclipse/che/security/oauth/GitLabOAuthAuthenticator.java b/...pi-auth-gitlab/src/main/java/org/eclipse/che/security/oauth/GitLabOAuthAuthenticator.java
@@ -91,11 +91,11 @@ protected <O> O getJson(String getUserUrl, String accessToken, Class<O> userClas
   public OAuthToken getToken(String userId) throws IOException {
     final OAuthToken token = super.getToken(userId);
     try {
-      if (token == null
-          || token.getToken() == null
-          || token.getToken().isEmpty()
-          || isNullOrEmpty(
-              getJson(gitlabUserEndpoint, token.getToken(), GitLabUser.class).getId())) {
+      if (token == null || token.getToken() == null || token.getToken().isEmpty()) {
+        return null;
+      }
+      GitLabUser user = getJson(gitlabUserEndpoint, token.getToken(), GitLabUser.class);
+      if (user == null || isNullOrEmpty(user.getId())) {
         return null;
       }
     } catch (OAuthAuthenticationException e) {

diff --git a/...api-auth-gitlab/src/test/java/org/eclipse/che/security/oauth/GitLabAuthenticatorTest.java b/...api-auth-gitlab/src/test/java/org/eclipse/che/security/oauth/GitLabAuthenticatorTest.java
@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) 2012-2024 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/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+package org.eclipse.che.security.oauth;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNull;
+
+import com.github.tomakehurst.wiremock.WireMockServer;
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.common.Slf4jNotifier;
+import com.google.api.client.auth.oauth2.StoredCredential;
+import com.google.api.client.util.store.MemoryDataStoreFactory;
+import com.google.common.net.HttpHeaders;
+import java.lang.reflect.Field;
+import org.eclipse.che.api.auth.shared.dto.OAuthToken;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class GitLabAuthenticatorTest {
+  WireMockServer wireMockServer;
+  WireMock wireMock;
+
+  @BeforeClass
+  public void setup() {
+    wireMockServer =
+        new WireMockServer(wireMockConfig().notifier(new Slf4jNotifier(false)).dynamicPort());
+    wireMockServer.start();
+    WireMock.configureFor("localhost", wireMockServer.port());
+    wireMock = new WireMock("localhost", wireMockServer.port());
+  }
+
+  @Test
+  public void shouldGetToken() throws Exception {
+    // given
+    GitLabOAuthAuthenticator gitLabOAuthAuthenticator =
+        new GitLabOAuthAuthenticator(
+            "id", "secret", wireMockServer.url("/"), "https://che.api.com");
+    Field flowField = OAuthAuthenticator.class.getDeclaredField("flow");
+    Field credentialDataStoreField =
+        ((Class) flowField.getGenericType()).getDeclaredField("credentialDataStore");
+    credentialDataStoreField.setAccessible(true);
+    credentialDataStoreField.set(
+        flowField.get(gitLabOAuthAuthenticator),
+        new MemoryDataStoreFactory()
+            .getDataStore("test")
+            .set("userId", new StoredCredential().setAccessToken("token")));
+    stubFor(
+        get(urlEqualTo("/api/v4/user"))
+            .withHeader(HttpHeaders.AUTHORIZATION, equalTo("Bearer token"))
+            .willReturn(aResponse().withBody("{\"id\": \"testId\"}")));
+    // when
+    OAuthToken token = gitLabOAuthAuthenticator.getToken("userId");
+    // then
+    assertEquals(token.getToken(), "token");
+  }
+
+  @Test
+  public void shouldGetEmptyToken() throws Exception {
+    // given
+    GitLabOAuthAuthenticator gitLabOAuthAuthenticator =
+        new GitLabOAuthAuthenticator(
+            "id", "secret", wireMockServer.url("/"), "https://che.api.com");
+    Field flowField = OAuthAuthenticator.class.getDeclaredField("flow");
+    Field credentialDataStoreField =
+        ((Class) flowField.getGenericType()).getDeclaredField("credentialDataStore");
+    credentialDataStoreField.setAccessible(true);
+    credentialDataStoreField.set(
+        flowField.get(gitLabOAuthAuthenticator),
+        new MemoryDataStoreFactory()
+            .getDataStore("test")
+            .set("userId", new StoredCredential().setAccessToken("token")));
+    stubFor(
+        get(urlEqualTo("/api/v4/user"))
+            .withHeader(HttpHeaders.AUTHORIZATION, equalTo("Bearer token"))
+            .willReturn(aResponse().withBody("{}")));
+    // when
+    OAuthToken token = gitLabOAuthAuthenticator.getToken("userId");
+    // then
+    assertNull(token);
+  }
+}
diff --git a/...er/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/OAuthAuthenticator.java b/...er/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/OAuthAuthenticator.java
@@ -285,7 +285,8 @@ protected <O> O getJson(String getUserUrl, String accessToken, Class<O> userClas
    *     none token found for user then {@code null} will be returned, when user have expired token
    *     and it can't be refreshed then {@code null} will be returned
    * @throws IOException when error occurs during token loading
-   * @see OAuthTokenProvider#getToken(String, String)
+   * @see OAuthTokenProvider#getToken(String, String) TODO: return Optional<OAuthToken> to avoid
+   *     returning null.
    */
   public OAuthToken getToken(String userId) throws IOException {
     if (!isConfigured()) {