From 08bede8f90e0fa75644e2d56e980ce359851bdeb Mon Sep 17 00:00:00 2001 From: Lukasz Leszczuk Date: Fri, 6 Sep 2019 16:52:17 +0200 Subject: [PATCH] Remove redundant debug logs. Fix nonce setting. Fix ordering in azure docs. --- Makefile | 2 +- docs/2_auth.md | 16 ++++----- oauthproxy.go | 6 +--- options.go | 10 +++--- providers/azure.go | 64 +++++++---------------------------- providers/internal_util.go | 10 ------ providers/logingov.go | 12 ------- providers/provider_default.go | 14 +++++++- 8 files changed, 40 insertions(+), 94 deletions(-) diff --git a/Makefile b/Makefile index 5f3612f..cba662a 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ include .env BINARY := oauth2_proxy -REPOSITORY := quay.io/pusher +REPOSITORY ?= quay.io/pusher VERSION := $(shell git describe --always --dirty --tags 2>/dev/null || echo "undefined") .NOTPARALLEL: diff --git a/docs/2_auth.md b/docs/2_auth.md index e251211..5133667 100644 --- a/docs/2_auth.md +++ b/docs/2_auth.md @@ -67,14 +67,14 @@ Note: The user is checked against the group members list on initial authenticati ### Azure Auth Provider 1. Add an application: go to [https://portal.azure.com](https://portal.azure.com), choose **"Azure Active Directory"** in the left menu, select **"App registrations"** and then click on **"New app registration"**. -1. Pick a name and choose **"Webapp / API"** as application type. Use `https://internal.yourcompany.com` as Sign-on URL. Click **"Create"**. -1. On the **"Overview"** page of the app read `client id (application id)` and `tenant id` -1. On the **"Manage"** / **"Authentication"** page of the app, pick a logo and select **"Multi-tenanted"** if you want to allow users from multiple organizations to access your app. Note down the application ID. Click **"Save"**. -1. On the **"Manage"** / **"Authentication"** page of the app, add `https://internal.yourcompanycom/oauth2/callback` for each host that you want to protect by the oauth2 proxy. Click **"Save"**. -1. On the **"Manage"** / **"API Permissions"** page of the app, click on **"Add a permission"** and then on **"Microsoft Graph"**/**"Delegated permissions"**/**"User"**/**"User.Read"**. Hit **"Add permissions"**. -1. On the **"Manage"** / **"Certificates & secret"** page of the app, add a new client secret, select expiration date and note down the value after hitting **"Add"** (it won't be readable after page reloads). -1. On the **"Manage"** / **"Manifest"** set `groupMembershipClaims` property to `SecurityGroup` -1. Configure the proxy with +2. Pick a name and choose **"Webapp / API"** as application type. Use `https://internal.yourcompany.com` as Sign-on URL. Click **"Create"**. +3. On the **"Overview"** page of the app read `client id (application id)` and `tenant id` +4. On the **"Manage"** / **"Authentication"** page of the app, pick a logo and select **"Multi-tenanted"** if you want to allow users from multiple organizations to access your app. Note down the application ID. Click **"Save"**. +5. On the **"Manage"** / **"Authentication"** page of the app, add `https://internal.yourcompanycom/oauth2/callback` for each host that you want to protect by the oauth2 proxy. Click **"Save"**. +6. On the **"Manage"** / **"API Permissions"** page of the app, click on **"Add a permission"** and then on **"Microsoft Graph"**/**"Delegated permissions"**/**"User"**/**"User.Read"**. Hit **"Add permissions"**. +7. On the **"Manage"** / **"Certificates & secret"** page of the app, add a new client secret, select expiration date and note down the value after hitting **"Add"** (it won't be readable after page reloads). +8. On the **"Manage"** / **"Manifest"** set `groupMembershipClaims` property to `SecurityGroup` +9. Configure the proxy with ``` --provider=azure diff --git a/oauthproxy.go b/oauthproxy.go index de2cd4d..1b7b85d 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -338,11 +338,7 @@ func (p *OAuthProxy) redeemCode(host, code string) (s *sessionsapi.SessionState, return nil, err } s.Email = userDetails.Email - if userDetails.UID != "" { - s.ID = userDetails.UID - } else { - s.ID = "" - } + s.ID = userDetails.UID } if s.User == "" { diff --git a/options.go b/options.go index d7b98bc..ab7eeea 100644 --- a/options.go +++ b/options.go @@ -69,10 +69,10 @@ type Options struct { SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens" env:"OAUTH2_PROXY_SKIP_JWT_BEARER_TOKENS"` ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers" env:"OAUTH2_PROXY_EXTRA_JWT_ISSUERS"` PassBasicAuth bool `flag:"pass-basic-auth" cfg:"pass_basic_auth" env:"OAUTH2_PROXY_PASS_BASIC_AUTH"` - PassGroups bool `flag:"pass-groups" cfg:"pass_groups"` - FilterGroups string `flag:"filter-groups" cfg:"filter_groups"` - PermitGroups []string `flag:"permit-groups" cfg:"permit_groups"` - GroupsDelimiter string `flag:"groups-delimiter" cfg:"groups_delimiter"` + PassGroups bool `flag:"pass-groups" cfg:"pass_groups" env:"OAUTH2_PROXY_PASS_GROUPS"` + FilterGroups string `flag:"filter-groups" cfg:"filter_groups" env:"OAUTH2_PROXY_FILTER_GROUPS"` + PermitGroups []string `flag:"permit-groups" cfg:"permit_groups" env:"OAUTH2_PROXY_PERMIT_GROUPS"` + GroupsDelimiter string `flag:"groups-delimiter" cfg:"groups_delimiter" env:"OAUTH2_PROXY_GROUPS_DELIMITER"` PermitUsers []string `flag:"permit-users" cfg:"permit_users"` BasicAuthPassword string `flag:"basic-auth-password" cfg:"basic_auth_password" env:"OAUTH2_PROXY_BASIC_AUTH_PASSWORD"` PassAccessToken bool `flag:"pass-access-token" cfg:"pass_access_token" env:"OAUTH2_PROXY_PASS_ACCESS_TOKEN"` @@ -168,7 +168,6 @@ func NewOptions() *Options { FilterGroups: "", GroupsDelimiter: "|", PermitGroups: []string{}, - PermitUsers: []string{}, PassAccessToken: false, PassHostHeader: true, SetAuthorization: false, @@ -409,7 +408,6 @@ func parseProviderInfo(o *Options, msgs []string) []string { switch p := o.provider.(type) { case *providers.AzureProvider: p.Configure(o.AzureTenant) - logger.Printf("PermitGroups %+v\n", splittedGroups) if len(splittedGroups) > 0 { p.SetGroupRestriction(splittedGroups) } diff --git a/providers/azure.go b/providers/azure.go index 95244c8..febc2a6 100644 --- a/providers/azure.go +++ b/providers/azure.go @@ -47,8 +47,6 @@ func NewAzureProvider(p *ProviderData) *AzureProvider { if p.ApprovalPrompt == "force" { p.ApprovalPrompt = "consent" } - logger.Printf("Approval prompt: '%s'", p.ApprovalPrompt) - return &AzureProvider{ProviderData: p} } @@ -115,7 +113,6 @@ func getUserIDFromJSON(json *simplejson.Json) (string, error) { if err != nil { return "", err } - return uid, err } @@ -132,32 +129,20 @@ func (p *AzureProvider) GetUserDetails(s *sessions.SessionState) (*UserDetails, req.Header = getAzureHeader(s.AccessToken) json, err := requests.Request(req) - if err != nil { return nil, err } - logger.Printf(" JSON: %v", json) - for key, value := range json.Interface().(map[string]interface{}) { - logger.Printf("\t %20v : %v", key, value) - } email, err := getEmailFromJSON(json) - if err != nil { - logger.Printf("[GetUserDetails] failed making request: %s", err) return nil, err } - uid, err := getUserIDFromJSON(json) - if err != nil { - logger.Printf("[GetUserDetails] failed to get User ID: %s", err) - } + uid, _ := getUserIDFromJSON(json) if email == "" { - logger.Printf("failed to get email address") return nil, errors.New("Client email not found") } - logger.Printf("[GetUserDetails] Chosen email address: '%s'", email) return &UserDetails{ Email: email, UID: uid, @@ -192,10 +177,8 @@ func (p *AzureProvider) GetGroups(s *sessions.SessionState, f string) (map[strin // ValidateExemptions checks if we can allow user login dispite group membership returned failure func (p *AzureProvider) ValidateExemptions(s *sessions.SessionState) (bool, string) { - logger.Printf("ValidateExemptions: validating for %v : %v", s.Email, s.ID) for eAccount, eGroup := range p.ExemptedUsers { if eAccount == s.Email || eAccount == s.Email+":"+s.ID { - logger.Printf("ValidateExemptions: \t found '%v' user in exemption list. Returning '%v' group membership", eAccount, eGroup) return true, eGroup } } @@ -213,7 +196,7 @@ func (p *AzureProvider) GetLoginURL(redirectURI, state string) string { params.Add("scope", p.Scope) params.Add("state", state) params.Set("prompt", p.ApprovalPrompt) - params.Set("nonce", "FIXME") + params.Set("nonce", randSeq(32)) if p.ProtectedResource != nil && p.ProtectedResource.String() != "" { params.Add("resource", p.ProtectedResource.String()) } @@ -230,25 +213,16 @@ func (p *AzureProvider) SetGroupRestriction(groups []string) { if len(groups) == 0 { return } - logger.Printf("Set group restrictions. Allowed groups are:") - logger.Printf("\t *GROUP NAME* : *GROUP ID*") for _, pGroup := range groups { splittedGroup := strings.Split(pGroup, ":") - var groupName string - var groupID string - if len(splittedGroup) == 1 { - groupName, groupID = splittedGroup[0], "" p.PermittedGroups[splittedGroup[0]] = "" - } else if len(splittedGroup) > 2 { - logger.Fatalf("failed to parse '%v'. Too many ':' separators", pGroup) - } else { - groupName, groupID = splittedGroup[0], splittedGroup[1] + } else if len(splittedGroup) == 2 { p.PermittedGroups[splittedGroup[0]] = splittedGroup[1] + } else { + logger.Printf("Warning: failed to parse '%v'. Too many ':' separators", pGroup) } - logger.Printf("\t - %-30s %s", groupName, groupID) } - logger.Printf("") } func (p *AzureProvider) SetGroupsExemption(exemptions []string) { @@ -262,8 +236,6 @@ func (p *AzureProvider) SetGroupsExemption(exemptions []string) { var userRecord string var groupName string - logger.Printf("Configure user exemption list:") - logger.Printf("\t *USER NAME*:*USER ID* : *DEFAULT GROUP*") for _, pRecord := range exemptions { splittedRecord := strings.Split(pRecord, ":") @@ -271,30 +243,24 @@ func (p *AzureProvider) SetGroupsExemption(exemptions []string) { userRecord, groupName = splittedRecord[0], "" } else if len(splittedRecord) == 2 { userRecord, groupName = splittedRecord[0], splittedRecord[1] - } else if len(splittedRecord) > 3 { - logger.Fatalf("failed to parse '%v'. Too many ':' separators", pRecord) } else { userRecord = splittedRecord[0] + ":" + splittedRecord[1] groupName = splittedRecord[2] } p.ExemptedUsers[userRecord] = groupName - logger.Printf("\t - %-65s %s", userRecord, groupName) } - logger.Printf("") } func (p *AzureProvider) ValidateGroupWithSession(s *sessions.SessionState) bool { - if len(p.PermittedGroups) != 0 { - for groupName, groupID := range p.PermittedGroups { - logger.Printf("ValidateGroup: %v", groupName) - if strings.Contains(s.Groups, groupID) { - return true - } - } - logger.Printf("Returning False from ValidateGroup") - return false + if len(p.PermittedGroups) == 0 { + return true } - return true + for _, groupID := range p.PermittedGroups { + if strings.Contains(s.Groups, groupID) { + return true + } + } + return false } func (p *AzureProvider) GroupPermitted(gName *string, gID *string) bool { @@ -306,15 +272,11 @@ func (p *AzureProvider) GroupPermitted(gName *string, gID *string) bool { if len(p.PermittedGroups) != 0 { for pGroupName, pGroupID := range p.PermittedGroups { if pGroupName == *gName { - logger.Printf("ValidateGroup: %v : %v", pGroupName, pGroupID) if pGroupID == "" || gID == nil { - logger.Printf("ValidateGroup: %v : %v : no Group ID defined for permitted group. Approving", pGroupName, pGroupID) return true } else if pGroupID == *gID { - logger.Printf("ValidateGroup: %v : %v : Group ID matches defined in permitted group. Approving", pGroupName, pGroupID) return true } - logger.Printf("ValidateGroup: %v : %v != %v Group IDs didn't match", pGroupName, pGroupID, *gID) } } return false diff --git a/providers/internal_util.go b/providers/internal_util.go index d345a50..d5a2bf6 100644 --- a/providers/internal_util.go +++ b/providers/internal_util.go @@ -13,16 +13,6 @@ type NotImplementedError struct { message string } -func NewNotImplementedError(message string) *NotImplementedError { - return &NotImplementedError{ - message: "Not implemented: " + message, - } -} - -func (e *NotImplementedError) Error() string { - return e.message -} - // stripToken is a helper function to obfuscate "access_token" // query parameters func stripToken(endpoint string) string { diff --git a/providers/logingov.go b/providers/logingov.go index 95d7aa9..30e88e3 100644 --- a/providers/logingov.go +++ b/providers/logingov.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "io/ioutil" - "math/rand" "net/http" "net/url" "time" @@ -29,17 +28,6 @@ type LoginGovProvider struct { PubJWKURL *url.URL } -// For generating a nonce -var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") - -func randSeq(n int) string { - b := make([]rune, n) - for i := range b { - b[i] = letters[rand.Intn(len(letters))] - } - return string(b) -} - // NewLoginGovProvider initiates a new LoginGovProvider func NewLoginGovProvider(p *ProviderData) *LoginGovProvider { p.ProviderName = "login.gov" diff --git a/providers/provider_default.go b/providers/provider_default.go index 6d84a43..497a5fc 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io/ioutil" + "math/rand" "net/http" "net/url" "time" @@ -95,6 +96,17 @@ func (p *ProviderData) GetLoginURL(redirectURI, state string) string { return a.String() } +// For generating a nonce +var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") + +func randSeq(n int) string { + b := make([]rune, n) + for i := range b { + b[i] = letters[rand.Intn(len(letters))] + } + return string(b) +} + // CookieForSession serializes a session state for storage in a cookie func (p *ProviderData) CookieForSession(s *sessions.SessionState, c *encryption.Cipher) (string, error) { return s.EncodeSessionState(c) @@ -111,7 +123,7 @@ func (p *ProviderData) GetEmailAddress(s *sessions.SessionState) (string, error) } func (p *ProviderData) GetUserDetails(s *sessions.SessionState) (*UserDetails, error) { - return nil, NewNotImplementedError("") + return nil, errors.New("not implemented") } // GetUserName returns the Account username