README.md

@@ -120,7 +120,6 @@ GITLAB_OMNIBUS_CONFIG: |
                 'scope' => ['openid', 'profile', 'email'],
                 'response_type' => 'code',
                 'issuer' => 'http://${oidc-provider}:${oidc-provider-port:-5556}/dex',
-                'client_auth_method' => 'query',
                 'discovery' => false, # GitLab discovery expects HTTPS
                 'uid_field' => 'preferred_username',
                 'client_options' => {
@@ -137,6 +136,21 @@ GITLAB_OMNIBUS_CONFIG: |
     ]
 ```
 
+Note that for the [OpenID-Connect spec](https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication)
+`basic` is the default client auth method if no different mechanism is
+configured at the client registration time.
+
+Quoting the [GitLab documentation](https://docs.gitlab.com/ee/administration/auth/oidc.html):
+
+> - `client_auth_method` (optional) specifies the method used for authenticating
+>   the client with the OpenID Connect provider.
+>   - Supported values are:
+>     - `basic` - HTTP Basic Authentication.
+>     - `jwt_bearer` - JWT-based authentication (private key and client secret signing).
+>     - `mtls` - Mutual TLS or X.509 certificate validation.
+>     - Any other value posts the client ID and secret in the request body.
+>   - If not specified, this value defaults to `basic`.
+
 Remember to open `80` port and [create API token](https://docs.gitlab.com/ee/user/profile/personal_access_tokens.html#create-a-personal-access-token-programmatically) for later use by the AuthZ mediator.
 
 ### AuthZ mediator

docker-compose-gitlab.yaml

@@ -19,6 +19,8 @@ services:
               'scope' => ['openid', 'profile', 'email'],
               'response_type' => 'code',
               'issuer' => 'http://oidc-provider:5556/dex',
+              # opt-in to client_secret_post client auth for no particular reason
+              # https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication
               'client_auth_method' => 'query',
               'discovery' => false, # GitLab discovery expects HTTPS
               'uid_field' => 'preferred_username',

docker-compose-qa.yaml

+version: "3.3"
+services:
+  qa-report-app:
+    image: registry.gitlab.apertis.org/infrastructure/qa-report-app/runtime:latest
+    ports:
+      - 28080:28080
+    environment:
+      DB_USER: &postgres_user qa-report-app
+      DB_PASSWORD: &postgres_password p4ssw0rd
+      DB_NAME: &postgres_db test_results
+      DB_HOST: postgres
+      DB_PORT: 5432
+      FLASK_SECRET_KEY: s3kr3t
+    volumes:
+      - type: bind
+        source: ./qa-config.yaml
+        target: /app/secrets/config.yaml
+
+  postgres:
+    image: postgres:15
+    environment:
+      POSTGRES_USER: *postgres_user
+      POSTGRES_PASSWORD: *postgres_password
+      POSTGRES_DB: *postgres_db
+
+  oidc-provider:
+    image: dexidp/dex:v2.40.0-alpine
+    security_opt:
+      - label=disable
+    ports:
+      - '5556:5556'
+    volumes:
+      - type: bind
+        source: ./dex.config.tmpl
+        target: //etc/dex/config.docker.yaml
+    environment:
+      DEX_ISSUER: &dex_issuer http://oidc-provider:5556/dex
+      DEX_CONNECTORS_ENABLE_LDAP: "true"
+      DEX_LDAP_HOST: ldap:389
+      DEX_LOG_LEVEL: debug
+      DEX_STATIC_CLIENT_ID: &id authz-mediator
+      DEX_STATIC_CLIENT_REDIRECT_URI: http://mediator:5555/callback
+      DEX_STATIC_CLIENT_NAME: 'Authorization mediator'
+      DEX_STATIC_CLIENT_SECRET: &dex_secret s3kr3t
+
+  ldap:
+    image: osixia/openldap:1.5.0
+    ports:
+      - '3890:389'
+    command: ["--copy-service"]
+    security_opt:
+      - label=disable
+    volumes:
+      - type: bind
+        source: ./config-ldap.ldif
+        target: /container/service/slapd/assets/config/bootstrap/ldif/custom/config-ldap.ldif
+
+  mediator:
+    image: "${AUTHZ_MEDIATOR_IMAGE:-authz-mediator}"
+    restart: on-failure
+    ports:
+      - '5555:5555'
+    command: authz-mediator -groups # explicit groups scope request; highest precedence - cannot be overriden
+    environment:
+      AUTHZ_MEDIATOR_SECRET: *dex_secret
+      AUTHZ_MEDIATOR_ID: *id
+      AUTHZ_MEDIATOR_BACKEND: none
+      AUTHZ_MEDIATOR_SELF: http://mediator:5555
+      AUTHZ_MEDIATOR_PORT: 5555
+      AUTHZ_MEDIATOR_PROVIDER: *dex_issuer
+      AUTHZ_MEDIATOR_INCLUDED_GROUPS: "maintainers developers admins"
+      AUTHZ_MEDIATOR_REDIRECT: http://qa-report-app:28080/openid_callback

mediator/handlers.go

@@ -92,15 +92,19 @@ func (med *Mediator) TokenHandler(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, formatJsonError("invalid_request", "missing %s param", CodeParam), http.StatusBadRequest)
 		return
 	}
-	clientID := r.FormValue(ClientIDParam)
-	if clientID == "" {
-		http.Error(w, formatJsonError("invalid_request", "missing %s param", ClientIDParam), http.StatusBadRequest)
-		return
-	}
-	clientSecret := r.FormValue(ClientSecretParam)
-	if clientSecret == "" {
-		http.Error(w, formatJsonError("invalid_request", "missing %s param", ClientSecretParam), http.StatusBadRequest)
-		return
+
+	clientID, clientSecret, ok := r.BasicAuth()
+	if !ok {
+		clientID = r.FormValue(ClientIDParam)
+		if clientID == "" {
+			http.Error(w, formatJsonError("invalid_request", "missing %s param", ClientIDParam), http.StatusBadRequest)
+			return
+		}
+		clientSecret = r.FormValue(ClientSecretParam)
+		if clientSecret == "" {
+			http.Error(w, formatJsonError("invalid_request", "missing %s param", ClientSecretParam), http.StatusBadRequest)
+			return
+		}
 	}
 
 	if clientID != med.config.ID {

mediator/mediator_test.go

@@ -295,7 +295,7 @@ var _ = Describe("Mediator", func() {
 			})
 		})
 
-		Context("when OIDC request has invalid client ID", func() {
+		Context("when OIDC request has invalid client ID in body", func() {
 			BeforeEach(func() {
 				vals.Set(mediator.ClientIDParam, "invalidID")
 				reader = strings.NewReader(vals.Encode())
@@ -311,7 +311,22 @@ var _ = Describe("Mediator", func() {
 			})
 		})
 
-		Context("when OIDC request has invalid client secret", func() {
+		Context("when OIDC request has invalid client ID in headers", func() {
+			BeforeEach(func() {
+				req = httptest.NewRequest(http.MethodPost, mediatorURL+mediator.TokenRoute, reader)
+				req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+				req.SetBasicAuth("invalidID", notSecret)
+				handler = http.HandlerFunc(med.TokenHandler)
+				handler.ServeHTTP(rr, req)
+			})
+
+			It("should return bad request error status code", func() {
+				Expect(rr.Code).To(Equal(http.StatusBadRequest))
+				Expect(rr.Body.String()).To(BeEquivalentTo("{\"error\":\"invalid_client\",\"error_description\":\"invalid client_id param\"}\n"))
+			})
+		})
+
+		Context("when OIDC request has invalid client secret in body", func() {
 			BeforeEach(func() {
 				vals.Set(mediator.ClientSecretParam, "invalidSecret")
 				reader = strings.NewReader(vals.Encode())
@@ -327,6 +342,21 @@ var _ = Describe("Mediator", func() {
 			})
 		})
 
+		Context("when OIDC request has invalid client secret in headers", func() {
+			BeforeEach(func() {
+				req = httptest.NewRequest(http.MethodPost, mediatorURL+mediator.TokenRoute, reader)
+				req.SetBasicAuth(mediatorID, "invalidSecret")
+				req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+				handler = http.HandlerFunc(med.TokenHandler)
+				handler.ServeHTTP(rr, req)
+			})
+
+			It("should return bad request error status code", func() {
+				Expect(rr.Code).To(Equal(http.StatusBadRequest))
+				Expect(rr.Body.String()).To(BeEquivalentTo("{\"error\":\"invalid_client\",\"error_description\":\"invalid client_secret param\"}\n"))
+			})
+		})
+
 		Context("when OIDC request cannot be made", func() {
 			BeforeEach(func() {
 				c.Provider = "" // request will return an error without protocol scheme
@@ -351,47 +381,67 @@ var _ = Describe("Mediator", func() {
 				tokens *mediator.TokenResponse
 			)
 
-			BeforeEach(func() {
-				tokens = &mediator.TokenResponse{
-					TokenType:   "bearer",
-					AccessToken: "granted",
-					IDToken:     "unknown",
-					ExpiresIn:   31337,
-				}
-
-				server = ghttp.NewServer()
-				server.AppendHandlers(
-					ghttp.CombineHandlers(
-						ghttp.VerifyRequest(http.MethodPost, "/"),
-						ghttp.RespondWithJSONEncoded(http.StatusOK, tokens),
-					),
-				)
-				e.Token = server.URL()
-				med, err = mediator.NewMediator(notSecret, notSecret, http.DefaultClient, &mediator.FakeVerifier{}, e, c)
-				Expect(err).ToNot(HaveOccurred())
-
-				req = httptest.NewRequest(http.MethodPost, mediatorURL+mediator.TokenRoute, reader)
-				req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-				handler = http.HandlerFunc(med.TokenHandler)
-				handler.ServeHTTP(rr, req)
-			})
-
-			AfterEach(func() {
-				server.Close()
-			})
-
-			It("should return OK status code", func() {
-				Expect(server.ReceivedRequests()).To(HaveLen(1))
-				Expect(rr.Code).To(Equal(http.StatusOK))
-			})
+			const (
+				authBasic    = "client_secret_basic"
+				authPostBody = "client_secret_post"
+			)
 
-			It("should pass server response unchanged", func() {
-				Expect(server.ReceivedRequests()).To(HaveLen(1))
-				tokens.ExpiresIn -= 1 // unless server response is near instantenous MatchJSON below will have to be reimplemented to fuzzy-match expiry time
-				tokenJSON, err := json.Marshal(tokens)
-				Expect(err).ToNot(HaveOccurred())
-				Expect(rr.Body.String()).To(MatchJSON(tokenJSON))
-			})
+			for _, authMethod := range []string{authBasic, authPostBody} {
+				authMethod := authMethod
+				Context("using authentication method "+authMethod, func() {
+					BeforeEach(func() {
+						tokens = &mediator.TokenResponse{
+							TokenType:   "bearer",
+							AccessToken: "granted",
+							IDToken:     "unknown",
+							ExpiresIn:   31337,
+						}
+
+						server = ghttp.NewServer()
+						server.AppendHandlers(
+							ghttp.CombineHandlers(
+								ghttp.VerifyRequest(http.MethodPost, "/"),
+								ghttp.RespondWithJSONEncoded(http.StatusOK, tokens),
+							),
+						)
+						e.Token = server.URL()
+						med, err = mediator.NewMediator(notSecret, notSecret, http.DefaultClient, &mediator.FakeVerifier{}, e, c)
+						Expect(err).ToNot(HaveOccurred())
+
+						if authMethod != authPostBody {
+							vals.Del(mediator.ClientIDParam)
+							vals.Del(mediator.ClientSecretParam)
+							reader = strings.NewReader(vals.Encode())
+						}
+
+						req = httptest.NewRequest(http.MethodPost, mediatorURL+mediator.TokenRoute, reader)
+						req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+						if authMethod == authBasic {
+							req.SetBasicAuth(mediatorID, notSecret)
+						}
+
+						handler = http.HandlerFunc(med.TokenHandler)
+						handler.ServeHTTP(rr, req)
+					})
+
+					AfterEach(func() {
+						server.Close()
+					})
+
+					It("should return OK status code", func() {
+						Expect(server.ReceivedRequests()).To(HaveLen(1))
+						Expect(rr.Code).To(Equal(http.StatusOK))
+					})
+
+					It("should pass server response unchanged", func() {
+						Expect(server.ReceivedRequests()).To(HaveLen(1))
+						tokens.ExpiresIn -= 1 // unless server response is near instantenous MatchJSON below will have to be reimplemented to fuzzy-match expiry time
+						tokenJSON, err := json.Marshal(tokens)
+						Expect(err).ToNot(HaveOccurred())
+						Expect(rr.Body.String()).To(MatchJSON(tokenJSON))
+					})
+				})
+			}
 		})
 	})
 

qa-config.yaml

+debug: True
+bug-tracking: none
+port: 28080
+auth-groups:
+  - name: AdminS
+    extra-perms:
+    - submit-reports
+    - tag-images
+  - name: developerS
+openid:
+  client-id: authz-mediator
+  client-secret: s3kr3t
+  manual:
+    issuer: http://oidc-provider:5556/dex
+    jwks-uri: http://mediator:5555/keys
+    authorization-endpoint: http://mediator:5555/auth
+    token-endpoint: http://mediator:5555/token
+    userinfo-endpoint: http://mediator:5555/userinfo