Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: filter sso groups based on regex #11774

Merged
merged 11 commits into from
Sep 10, 2023
+59 −28
Merged

feat: filter sso groups based on regex #11774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d8dfc5a
feat: filter sso groups based on regex
Sep 8, 2023
44b84a3
ci: empty commit for build
Sep 8, 2023
38c2b77
feat: pre initialize regex
Sep 8, 2023
e15387a
ci: empty commit for build
Sep 8, 2023
1dfd1eb
feat: make filterGroupRegex a list
Sep 8, 2023
b1ea29b
feat: make filterGroupRegex a list
Sep 8, 2023
e469614
ci: empty commit for build
Sep 9, 2023
157b137
ci: empty commit for build
Sep 9, 2023
4194ac8
fix: stylistic issues
Sep 9, 2023
6b6742b
fix: stylistic issues
Sep 9, 2023
3358f57
feat: log filterGroupsRegex
Sep 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions config/sso.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ type SSOConfig struct {
Scopes []string `json:"scopes,omitempty"`
SessionExpiry metav1.Duration `json:"sessionExpiry,omitempty"`
// customGroupClaimName will override the groups claim name
CustomGroupClaimName string `json:"customGroupClaimName,omitempty"`
UserInfoPath string `json:"userInfoPath,omitempty"`
InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"`
CustomGroupClaimName string `json:"customGroupClaimName,omitempty"`
UserInfoPath string `json:"userInfoPath,omitempty"`
InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"`
FilterGroupsRegex []string `json:"filterGroupsRegex,omitempty"`
}

func (c SSOConfig) GetSessionExpiry() time.Duration {
Expand Down
80 changes: 55 additions & 25 deletions server/auth/sso/sso.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"crypto/x509"
"fmt"
"net/http"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -49,18 +50,19 @@ var _ Interface = &sso{}
type Config = config.SSOConfig

type sso struct {
config *oauth2.Config
issuer string
idTokenVerifier *oidc.IDTokenVerifier
httpClient *http.Client
baseHRef string
secure bool
privateKey crypto.PrivateKey
encrypter jose.Encrypter
rbacConfig *config.RBACConfig
expiry time.Duration
customClaimName string
userInfoPath string
config *oauth2.Config
issuer string
idTokenVerifier *oidc.IDTokenVerifier
httpClient *http.Client
baseHRef string
secure bool
privateKey crypto.PrivateKey
encrypter jose.Encrypter
rbacConfig *config.RBACConfig
expiry time.Duration
customClaimName string
userInfoPath string
filterGroupsRegex []*regexp.Regexp
}

func (s *sso) IsRBACEnabled() bool {
Expand Down Expand Up @@ -181,25 +183,38 @@ func newSso(
if err != nil {
return nil, fmt.Errorf("failed to create JWT encrpytor: %w", err)
}
lf := log.Fields{"redirectUrl": config.RedirectURL, "issuer": c.Issuer, "issuerAlias": "DISABLED", "clientId": c.ClientID, "scopes": config.Scopes, "insecureSkipVerify": c.InsecureSkipVerify}

var filterGroupsRegex []*regexp.Regexp
if c.FilterGroupsRegex != nil && len(c.FilterGroupsRegex) > 0 {
for _, regex := range c.FilterGroupsRegex {
compiledRegex, err := regexp.Compile(regex)
if err != nil {
return nil, fmt.Errorf("failed to compile sso.filterGroupRegex: %s %w", regex, err)
}
filterGroupsRegex = append(filterGroupsRegex, compiledRegex)
}
}

lf := log.Fields{"redirectUrl": config.RedirectURL, "issuer": c.Issuer, "issuerAlias": "DISABLED", "clientId": c.ClientID, "scopes": config.Scopes, "insecureSkipVerify": c.InsecureSkipVerify, "filterGroupsRegex": c.FilterGroupsRegex}
if c.IssuerAlias != "" {
lf["issuerAlias"] = c.IssuerAlias
}
log.WithFields(lf).Info("SSO configuration")
agilgur5 marked this conversation as resolved.
Show resolved Hide resolved

return &sso{
config: config,
idTokenVerifier: idTokenVerifier,
baseHRef: baseHRef,
httpClient: httpClient,
secure: secure,
privateKey: privateKey,
encrypter: encrypter,
rbacConfig: c.RBAC,
expiry: c.GetSessionExpiry(),
customClaimName: c.CustomGroupClaimName,
userInfoPath: c.UserInfoPath,
issuer: c.Issuer,
config: config,
idTokenVerifier: idTokenVerifier,
baseHRef: baseHRef,
httpClient: httpClient,
secure: secure,
privateKey: privateKey,
encrypter: encrypter,
rbacConfig: c.RBAC,
expiry: c.GetSessionExpiry(),
customClaimName: c.CustomGroupClaimName,
userInfoPath: c.UserInfoPath,
issuer: c.Issuer,
filterGroupsRegex: filterGroupsRegex,
}, nil
}

Expand Down Expand Up @@ -280,6 +295,21 @@ func (s *sso) HandleCallback(w http.ResponseWriter, r *http.Request) {
return
}
}

// only return groups that match at least one of the regexes
if s.filterGroupsRegex != nil && len(s.filterGroupsRegex) > 0 {
agilgur5 marked this conversation as resolved.
Show resolved Hide resolved
var filteredGroups []string
Copy link
Member

@agilgur5 agilgur5 Sep 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be great if the provider could do this logic so that the response from it is not so large, but it does not seem there is a uniform provider interface for that.

There is a related Dex issue on this (dexidp/dex#1476) and generalizing it was basically closed out due to provider-specific nuances.

So I think there may be no way around filtering in the Server code unfortunately.

If a user has an IdP proxy like Dex though, they can do this group filter logic within Dex. As we get into more complex scenarios like these, we may want to consider limiting the scope of the internal implementation and forwarding users to Dex et al instead.

for _, group := range groups {
for _, regex := range s.filterGroupsRegex {
if regex.MatchString(group) {
filteredGroups = append(filteredGroups, group)
break
}
}
}
groups = filteredGroups
}

argoClaims := &types.Claims{
Claims: jwt.Claims{
Issuer: issuer,
Expand Down