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.

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

feat: filter sso groups based on regex #11774

Merged
merged 11 commits into from
Sep 10, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions config/sso.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type SSOConfig struct {
CustomGroupClaimName string `json:"customGroupClaimName,omitempty"`
UserInfoPath string `json:"userInfoPath,omitempty"`
InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"`
FilterSSOGroupsRegex string `json:"filterSSOGroupsRegex,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
FilterSSOGroupsRegex string `json:"filterSSOGroupsRegex,omitempty"`
FilterGroupsRegex string `json:"filterGroupsRegex,omitempty"`

"SSO" is redundant here as this is a field of SSOConfig

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

func (c SSOConfig) GetSessionExpiry() time.Duration {
Expand Down
65 changes: 41 additions & 24 deletions server/auth/sso/sso.go
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
filterSsoGroupsRegex string
}

func (s *sso) IsRBACEnabled() bool {
Expand Down Expand Up @@ -188,18 +190,19 @@ func newSso(
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,
filterSsoGroupsRegex: c.FilterSSOGroupsRegex,
}, nil
}

Expand Down Expand Up @@ -280,6 +283,20 @@ func (s *sso) HandleCallback(w http.ResponseWriter, r *http.Request) {
return
}
}
if s.filterSsoGroupsRegex != "" {
var filteredGroups []string
Copy link
Member

@agilgur5 agilgur5 Sep 9, 2023

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.

regex, err := regexp.Compile(s.filterSsoGroupsRegex)
if err != nil {
log.WithError(err).Errorf("failed to compile filterSsoGroupsRegex: %s", s.filterSsoGroupsRegex)
agilgur5 marked this conversation as resolved.
Show resolved Hide resolved
} else {
for _, group := range groups {
if regex.Match([]byte(group)) {
filteredGroups = append(filteredGroups, group)
}
}
groups = filteredGroups
}
}
argoClaims := &types.Claims{
Claims: jwt.Claims{
Issuer: issuer,
Expand Down