Skip to content

Commit

Permalink
support multiple subjects in oidc ping (fatedier#4475)
Browse files Browse the repository at this point in the history
Resolves: fatedier#4466
  • Loading branch information
RobKenis authored Oct 12, 2024
1 parent 2855ac7 commit 2466e65
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 9 deletions.
3 changes: 2 additions & 1 deletion pkg/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ func NewAuthVerifier(cfg v1.AuthServerConfig) (authVerifier Verifier) {
case v1.AuthMethodToken:
authVerifier = NewTokenAuth(cfg.AdditionalScopes, cfg.Token)
case v1.AuthMethodOIDC:
authVerifier = NewOidcAuthVerifier(cfg.AdditionalScopes, cfg.OIDC)
tokenVerifier := NewTokenVerifier(cfg.OIDC)
authVerifier = NewOidcAuthVerifier(cfg.AdditionalScopes, tokenVerifier)
}
return authVerifier
}
27 changes: 19 additions & 8 deletions pkg/auth/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,18 @@ func (auth *OidcAuthProvider) SetNewWorkConn(newWorkConnMsg *msg.NewWorkConn) (e
return err
}

type TokenVerifier interface {
Verify(context.Context, string) (*oidc.IDToken, error)
}

type OidcAuthConsumer struct {
additionalAuthScopes []v1.AuthScope

verifier *oidc.IDTokenVerifier
subjectFromLogin string
verifier TokenVerifier
subjectsFromLogin []string
}

func NewOidcAuthVerifier(additionalAuthScopes []v1.AuthScope, cfg v1.AuthOIDCServerConfig) *OidcAuthConsumer {
func NewTokenVerifier(cfg v1.AuthOIDCServerConfig) TokenVerifier {
provider, err := oidc.NewProvider(context.Background(), cfg.Issuer)
if err != nil {
panic(err)
Expand All @@ -105,9 +109,14 @@ func NewOidcAuthVerifier(additionalAuthScopes []v1.AuthScope, cfg v1.AuthOIDCSer
SkipExpiryCheck: cfg.SkipExpiryCheck,
SkipIssuerCheck: cfg.SkipIssuerCheck,
}
return provider.Verifier(&verifierConf)
}

func NewOidcAuthVerifier(additionalAuthScopes []v1.AuthScope, verifier TokenVerifier) *OidcAuthConsumer {
return &OidcAuthConsumer{
additionalAuthScopes: additionalAuthScopes,
verifier: provider.Verifier(&verifierConf),
verifier: verifier,
subjectsFromLogin: []string{},
}
}

Expand All @@ -116,7 +125,9 @@ func (auth *OidcAuthConsumer) VerifyLogin(loginMsg *msg.Login) (err error) {
if err != nil {
return fmt.Errorf("invalid OIDC token in login: %v", err)
}
auth.subjectFromLogin = token.Subject
if !slices.Contains(auth.subjectsFromLogin, token.Subject) {
auth.subjectsFromLogin = append(auth.subjectsFromLogin, token.Subject)
}
return nil
}

Expand All @@ -125,11 +136,11 @@ func (auth *OidcAuthConsumer) verifyPostLoginToken(privilegeKey string) (err err
if err != nil {
return fmt.Errorf("invalid OIDC token in ping: %v", err)
}
if token.Subject != auth.subjectFromLogin {
if !slices.Contains(auth.subjectsFromLogin, token.Subject) {
return fmt.Errorf("received different OIDC subject in login and ping. "+
"original subject: %s, "+
"original subjects: %s, "+
"new subject: %s",
auth.subjectFromLogin, token.Subject)
auth.subjectsFromLogin, token.Subject)
}
return nil
}
Expand Down
64 changes: 64 additions & 0 deletions pkg/auth/oidc_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package auth_test

import (
"context"
"testing"
"time"

"github.com/coreos/go-oidc/v3/oidc"
"github.com/stretchr/testify/require"

"github.com/fatedier/frp/pkg/auth"
v1 "github.com/fatedier/frp/pkg/config/v1"
"github.com/fatedier/frp/pkg/msg"
)

type mockTokenVerifier struct{}

func (m *mockTokenVerifier) Verify(ctx context.Context, subject string) (*oidc.IDToken, error) {
return &oidc.IDToken{
Subject: subject,
}, nil
}

func TestPingWithEmptySubjectFromLoginFails(t *testing.T) {
r := require.New(t)
consumer := auth.NewOidcAuthVerifier([]v1.AuthScope{v1.AuthScopeHeartBeats}, &mockTokenVerifier{})
err := consumer.VerifyPing(&msg.Ping{
PrivilegeKey: "ping-without-login",
Timestamp: time.Now().UnixMilli(),
})
r.Error(err)
r.Contains(err.Error(), "received different OIDC subject in login and ping")
}

func TestPingAfterLoginWithNewSubjectSucceeds(t *testing.T) {
r := require.New(t)
consumer := auth.NewOidcAuthVerifier([]v1.AuthScope{v1.AuthScopeHeartBeats}, &mockTokenVerifier{})
err := consumer.VerifyLogin(&msg.Login{
PrivilegeKey: "ping-after-login",
})
r.NoError(err)

err = consumer.VerifyPing(&msg.Ping{
PrivilegeKey: "ping-after-login",
Timestamp: time.Now().UnixMilli(),
})
r.NoError(err)
}

func TestPingAfterLoginWithDifferentSubjectFails(t *testing.T) {
r := require.New(t)
consumer := auth.NewOidcAuthVerifier([]v1.AuthScope{v1.AuthScopeHeartBeats}, &mockTokenVerifier{})
err := consumer.VerifyLogin(&msg.Login{
PrivilegeKey: "login-with-first-subject",
})
r.NoError(err)

err = consumer.VerifyPing(&msg.Ping{
PrivilegeKey: "ping-with-different-subject",
Timestamp: time.Now().UnixMilli(),
})
r.Error(err)
r.Contains(err.Error(), "received different OIDC subject in login and ping")
}

0 comments on commit 2466e65

Please sign in to comment.