fix: resilient social sign in

go.mod

@@ -78,7 +78,7 @@ require (
 	github.com/ory/jsonschema/v3 v3.0.7
 	github.com/ory/mail/v3 v3.0.0
 	github.com/ory/nosurf v1.2.7
-	github.com/ory/x v0.0.527
+	github.com/ory/x v0.0.531
 	github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2
 	github.com/pkg/errors v0.9.1
 	github.com/pquerna/otp v1.4.0

go.sum

@@ -1140,8 +1140,8 @@ github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2 h1:zm6sDvHy/U9XrGpi
 github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM=
 github.com/ory/viper v1.7.5 h1:+xVdq7SU3e1vNaCsk/ixsfxE4zylk1TJUiJrY647jUE=
 github.com/ory/viper v1.7.5/go.mod h1:ypOuyJmEUb3oENywQZRgeAMwqgOyDqwboO1tj3DjTaM=
-github.com/ory/x v0.0.527 h1:K6MmsYqT1NMb8VQ4hhn9q6NnrnecwNQJXc1bEoixQ8Y=
-github.com/ory/x v0.0.527/go.mod h1:XBqhPZRppPHTxtsE0l0oI/B2Onf1QJtMRGPh3CpEpA0=
+github.com/ory/x v0.0.531 h1:ndkhfzgX7y1ChNnYPS5ioqVuvyRENXKUBrNnkmoPOFQ=
+github.com/ory/x v0.0.531/go.mod h1:XBqhPZRppPHTxtsE0l0oI/B2Onf1QJtMRGPh3CpEpA0=
 github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw=
 github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE=
 github.com/otiai10/curr v1.0.0/go.mod h1:LskTG5wDwr8Rs+nNQ+1LlxRjAtTZZjtJW4rMXl6j4vs=

identity/credentials_oidc.go

@@ -33,6 +33,14 @@ type CredentialsOIDCProvider struct {
 
 // NewCredentialsOIDC creates a new OIDC credential.
 func NewCredentialsOIDC(idToken, accessToken, refreshToken, provider, subject string) (*Credentials, error) {
+	if provider == "" {
+		return nil, errors.New("received empty provider in oidc credentials")
+	}
+
+	if subject == "" {
+		return nil, errors.New("received empty provider in oidc credentials")
+	}
+
 	var b bytes.Buffer
 	if err := json.NewEncoder(&b).Encode(CredentialsOIDC{
 		Providers: []CredentialsOIDCProvider{

identity/credentials_oidc_test.go

@@ -0,0 +1,19 @@
+// Copyright © 2023 Ory Corp
+// SPDX-License-Identifier: Apache-2.0
+
+package identity
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewCredentialsOIDC(t *testing.T) {
+	_, err := NewCredentialsOIDC("", "", "", "", "not-empty")
+	require.Error(t, err)
+	_, err = NewCredentialsOIDC("", "", "", "not-empty", "")
+	require.Error(t, err)
+	_, err = NewCredentialsOIDC("", "", "", "not-empty", "not-empty")
+	require.NoError(t, err)
+}

selfservice/strategy/oidc/error.go

@@ -3,7 +3,15 @@
 
 package oidc
 
-import "github.com/ory/herodot"
+import (
+	"io"
+	"net/http"
+
+	"github.com/pkg/errors"
+
+	"github.com/ory/herodot"
+	"github.com/ory/x/logrusx"
+)
 
 var (
 	ErrScopeMissing = herodot.ErrBadRequest.
@@ -17,3 +25,17 @@ var (
 	ErrAPIFlowNotSupported = herodot.ErrBadRequest.WithError("API-based flows are not supported for this method").
 				WithReasonf("Social Sign In and OpenID Connect are only supported for flows initiated using the Browser endpoint.")
 )
+
+func logUpstreamError(l *logrusx.Logger, resp *http.Response) error {
+	if resp.StatusCode == http.StatusOK {
+		return nil
+	}
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		body = []byte(err.Error())
+	}
+
+	l.WithField("response_code", resp.StatusCode).WithField("response_body", string(body)).Error("The upstream OIDC provider returned a non 200 status code.")
+	return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("OpenID Connect provider returned a %d status code but 200 is expected.", resp.StatusCode))
+}

selfservice/strategy/oidc/provider.go

@@ -7,6 +7,10 @@ import (
 	"context"
 	"net/url"
 
+	"github.com/pkg/errors"
+
+	"github.com/ory/herodot"
+
 	"golang.org/x/oauth2"
 
 	"github.com/ory/kratos/x"
@@ -50,3 +54,14 @@ type Claims struct {
 	Team                string                 `json:"team,omitempty"`
 	RawClaims           map[string]interface{} `json:"raw_claims,omitempty"`
 }
+
+// Validate checks if the claims are valid.
+func (c *Claims) Validate() error {
+	if c.Subject == "" {
+		return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("provider did not return a subject"))
+	}
+	if c.Issuer == "" {
+		return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("issuer not set in claims"))
+	}
+	return nil
+}

selfservice/strategy/oidc/provider_auth0.go

@@ -87,7 +87,6 @@ func (g *ProviderAuth0) Claims(ctx context.Context, exchange *oauth2.Token, quer
 		return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("%s", err))
 	}
 
-	req.Header.Add("Authorization", "Bearer: "+exchange.AccessToken)
 	req.Header.Add("Content-Type", "application/json")
 
 	resp, err := client.Do(req)
@@ -96,6 +95,10 @@ func (g *ProviderAuth0) Claims(ctx context.Context, exchange *oauth2.Token, quer
 	}
 	defer resp.Body.Close()
 
+	if err := logUpstreamError(g.reg.Logger(), resp); err != nil {
+		return nil, err
+	}
+
 	// Once auth0 fixes this bug, all this workaround can be removed.
 	b, err := io.ReadAll(resp.Body)
 	if err != nil {

selfservice/strategy/oidc/provider_dingtalk.go

@@ -96,6 +96,10 @@ func (g *ProviderDingTalk) Exchange(ctx context.Context, code string, opts ...oa
 	}
 	defer resp.Body.Close()
 
+	if err := logUpstreamError(g.reg.Logger(), resp); err != nil {
+		return nil, err
+	}
+
 	var dToken struct {
 		ErrCode     int    `json:"code"`
 		ErrMsg      string `json:"message"`
@@ -135,6 +139,10 @@ func (g *ProviderDingTalk) Claims(ctx context.Context, exchange *oauth2.Token, _
 	}
 	defer resp.Body.Close()
 
+	if err := logUpstreamError(g.reg.Logger(), resp); err != nil {
+		return nil, err
+	}
+
 	var user struct {
 		Nick      string `json:"nick"`
 		OpenId    string `json:"openId"`

selfservice/strategy/oidc/provider_facebook.go

@@ -86,6 +86,10 @@ func (g *ProviderFacebook) Claims(ctx context.Context, exchange *oauth2.Token, q
 	}
 	defer resp.Body.Close()
 
+	if err := logUpstreamError(g.reg.Logger(), resp); err != nil {
+		return nil, err
+	}
+
 	var user struct {
 		Id            string `json:"id,omitempty"`
 		Name          string `json:"name,omitempty"`

selfservice/strategy/oidc/provider_gitlab.go

@@ -6,7 +6,6 @@ package oidc
 import (
 	"context"
 	"encoding/json"
-	"net/http"
 	"net/url"
 	"path"
 
@@ -93,8 +92,8 @@ func (g *ProviderGitLab) Claims(ctx context.Context, exchange *oauth2.Token, que
 	}
 	defer resp.Body.Close()
 
-	if resp.StatusCode != http.StatusOK {
-		return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Expected the GitLab userinfo endpoint to return a 200 OK response but got %d instead.", resp.StatusCode))
+	if err := logUpstreamError(g.reg.Logger(), resp); err != nil {
+		return nil, err
 	}
 
 	var claims Claims

selfservice/strategy/oidc/provider_microsoft.go

@@ -6,7 +6,6 @@ package oidc
 import (
 	"context"
 	"encoding/json"
-	"net/http"
 	"net/url"
 	"strings"
 
@@ -103,8 +102,8 @@ func (m *ProviderMicrosoft) updateSubject(ctx context.Context, claims *Claims, e
 		}
 		defer resp.Body.Close()
 
-		if resp.StatusCode != http.StatusOK {
-			return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to fetch from `https://graph.microsoft.com/v1.0/me: Got Status %s", resp.Status))
+		if err := logUpstreamError(m.reg.Logger(), resp); err != nil {
+			return nil, err
 		}
 
 		var user struct {

selfservice/strategy/oidc/provider_netid.go

@@ -86,6 +86,10 @@ func (n *ProviderNetID) Claims(ctx context.Context, exchange *oauth2.Token, _ ur
 	}
 	defer resp.Body.Close()
 
+	if err := logUpstreamError(n.reg.Logger(), resp); err != nil {
+		return nil, err
+	}
+
 	var claims Claims
 	if err := json.NewDecoder(resp.Body).Decode(&claims); err != nil {
 		return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("%s", err))

selfservice/strategy/oidc/provider_test.go

@@ -0,0 +1,20 @@
+// Copyright © 2023 Ory Corp
+// SPDX-License-Identifier: Apache-2.0
+
+package oidc
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestClaimsValidate(t *testing.T) {
+	require.Error(t, new(Claims).Validate())
+	require.Error(t, (&Claims{Issuer: "not-empty"}).Validate())
+	require.Error(t, (&Claims{Issuer: "not-empty"}).Validate())
+	require.Error(t, (&Claims{Subject: "not-empty"}).Validate())
+	require.Error(t, (&Claims{Subject: "not-empty"}).Validate())
+	require.NoError(t, (&Claims{Issuer: "not-empty", Subject: "not-empty"}).Validate())
+	require.NoError(t, (&Claims{Issuer: "not-empty", Subject: "not-empty"}).Validate())
+}