Remove redundant debug logs. Fix nonce setting. Fix ordering in azure docs.

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:
 

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"**.
+2. 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` 
+3. 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"**.
+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"**.
-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"**.
+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"**.
-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"**.
+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"**.
-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).
+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).
-1. On the **"Manage"** / **"Manifest"** set `groupMembershipClaims` property to `SecurityGroup`
+8. On the **"Manage"** / **"Manifest"** set `groupMembershipClaims` property to `SecurityGroup`
-1. Configure the proxy with
+9. Configure the proxy with
 
 ```
    --provider=azure

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
-			s.ID = userDetails.UID
-		} else {
-			s.ID = ""
-		}
 	}
 
 	if s.User == "" {

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"`
+	PassGroups                    bool          `flag:"pass-groups" cfg:"pass_groups" env:"OAUTH2_PROXY_PASS_GROUPS"`
-	FilterGroups                  string        `flag:"filter-groups" cfg:"filter_groups"`
+	FilterGroups                  string        `flag:"filter-groups" cfg:"filter_groups" env:"OAUTH2_PROXY_FILTER_GROUPS"`
-	PermitGroups                  []string      `flag:"permit-groups" cfg:"permit_groups"`
+	PermitGroups                  []string      `flag:"permit-groups" cfg:"permit_groups" env:"OAUTH2_PROXY_PERMIT_GROUPS"`
-	GroupsDelimiter               string        `flag:"groups-delimiter" cfg:"groups_delimiter"`
+	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)
 		}

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)
+	uid, _ := getUserIDFromJSON(json)
-	if err != nil {
-		logger.Printf("[GetUserDetails] failed to get User ID: %s", err)
-	}
 
 	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 {
+		} else if len(splittedGroup) == 2 {
-			logger.Fatalf("failed to parse '%v'. Too many ':' separators", pGroup)
-		} else {
-			groupName, groupID = splittedGroup[0], splittedGroup[1]
 			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 {
+	if len(p.PermittedGroups) == 0 {
-		for groupName, groupID := range p.PermittedGroups {
+		return true
-			logger.Printf("ValidateGroup: %v", groupName)
-			if strings.Contains(s.Groups, groupID) {
-				return true
-			}
-		}
-		logger.Printf("Returning False from ValidateGroup")
-		return false
 	}
-	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

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 {

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"

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