Repository: oauth2-proxy/oauth2-proxy
Pull request #1238 (2021-06-13 08:12:59)

Added ADFS Provider
## Description
Added ADFS provider to support Windows Server openid Web Sign On.

## Motivation and Context
We're planning to use aotuh2-proxy as one of the authentication systems for webapps for the company I'm working on. In order to achieve this goal, It will be very helpful to support ADFS since it's currently in use for many projects.

## How Has This Been Tested?

Wrote unit tests with the ginko library taking as examples gitlab_test.go and keycloak_test.go
Created an ADFS openid application and configured a dev environment pointing to the company test ADFS openid endpoint.
Tested regular user login with web-browser with SSO enabled and disabled.
Tested login with Authentication Bearer header and a programmatically generated token.
Created a new provider called adfs.
Created tests.
Added the new provider to the providers list.
Added documentation.
Since it's a different provider it shouldn't affect any other parts of the code or break compatibility.

## Checklist:

- [X] My change requires a change to the documentation or CHANGELOG.
- [X] I have updated the documentation/CHANGELOG accordingly.
- [X] I have created a feature (non-master) branch for my PR.


samirachoadi: Sorry @NickMeves I think I messed up the old pull request with the rebase. I just opened a new one.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e6a1c7ea..fd3d6d3c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@
 ## Breaking Changes
 
 ## Changes since v7.1.3
+- [#1238](https://github.com/oauth2-proxy/oauth2-proxy/pull/1238) Added ADFS provider (@samirachoadi)
 - [#1227](https://github.com/oauth2-proxy/oauth2-proxy/pull/1227) Fix Refresh Session not working for multiple cookies (@rishi1111)
 - [#1063](https://github.com/oauth2-proxy/oauth2-proxy/pull/1063) Add Redis lock feature to lock persistent sessions (@Bibob7)
 - [#1108](https://github.com/oauth2-proxy/oauth2-proxy/pull/1108) Add alternative ways to generate cookie secrets to docs (@JoelSpeed)
diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md
index ea2466d6..ca490b34 100644
--- a/docs/docs/configuration/alpha_config.md
+++ b/docs/docs/configuration/alpha_config.md
@@ -101,6 +101,16 @@ You must remove these options before starting OAuth2 Proxy with `--alpha-config`
 ## Configuration Reference
 <!--- THIS FILE IS AUTOGENERATED!!! DO NOT EDIT!!! -->
 
+### ADFSOptions
+
+(**Appears on:** [Provider](#provider))
+
+
+
+| Field | Type | Description |
+| ----- | ---- | ----------- |
+| `skipScope` | _bool_ | Skip adding the scope parameter in login request<br/>Default value is 'false' |
+
 ### AlphaOptions
 
 AlphaOptions contains alpha structured configuration options.
@@ -284,6 +294,7 @@ Provider holds all configuration for a single provider
 | `clientSecretFile` | _string_ | ClientSecretFile is the name of the file<br/>containing the OAuth Client Secret, it will be used if ClientSecret is not set. |
 | `keycloakConfig` | _[KeycloakOptions](#keycloakoptions)_ | KeycloakConfig holds all configurations for Keycloak provider. |
 | `azureConfig` | _[AzureOptions](#azureoptions)_ | AzureConfig holds all configurations for Azure provider. |
+| `ADFSConfig` | _[ADFSOptions](#adfsoptions)_ | ADFSConfig holds all configurations for ADFS provider. |
 | `bitbucketConfig` | _[BitbucketOptions](#bitbucketoptions)_ | BitbucketConfig holds all configurations for Bitbucket provider. |
 | `githubConfig` | _[GitHubOptions](#githuboptions)_ | GitHubConfig holds all configurations for GitHubC provider. |
 | `gitlabConfig` | _[GitLabOptions](#gitlaboptions)_ | GitLabConfig holds all configurations for GitLab provider. |
@@ -297,7 +308,7 @@ Provider holds all configuration for a single provider
 | `loginURL` | _string_ | LoginURL is the authentication endpoint |
 | `redeemURL` | _string_ | RedeemURL is the token redemption endpoint |
 | `profileURL` | _string_ | ProfileURL is the profile access endpoint |
-| `resource` | _string_ | ProtectedResource is the resource that is protected (Azure AD only) |
+| `resource` | _string_ | ProtectedResource is the resource that is protected (Azure AD and ADFS only) |
 | `validateURL` | _string_ | ValidateURL is the access token validation endpoint |
 | `scope` | _string_ | Scope is the OAuth scope specification |
 | `prompt` | _string_ | Prompt is OIDC prompt |
diff --git a/docs/docs/configuration/auth.md b/docs/docs/configuration/auth.md
index 6a667da0..0673c295 100644
--- a/docs/docs/configuration/auth.md
+++ b/docs/docs/configuration/auth.md
@@ -9,6 +9,7 @@ Valid providers are :
 
 - [Google](#google-auth-provider) _default_
 - [Azure](#azure-auth-provider)
+- [ADFS](#adfs-auth-provider)
 - [Facebook](#facebook-auth-provider)
 - [GitHub](#github-auth-provider)
 - [Keycloak](#keycloak-auth-provider)
@@ -88,6 +89,21 @@ Note: The user is checked against the group members list on initial authenticati
 
 Note: When using the Azure Auth provider with nginx and the cookie session store you may find the cookie is too large and doesn't get passed through correctly. Increasing the proxy_buffer_size in nginx or implementing the [redis session storage](sessions.md#redis-storage) should resolve this.
 
+### ADFS Auth Provider
+
+1. Open the ADFS administration console on your Windows Server and add a new Application Group
+2. Provide a name for the integration, select Server Application from the Standalone applications section and click Next
+3. Follow the wizard to get the client-id, client-secret and configure the application credentials
+4. Configure the proxy with
+
+```
+   --provider=adfs
+   --client-id=<application ID from step 3>
+   --client-secret=<value from step 3>
+```
+
+Note: When using the ADFS Auth provider with nginx and the cookie session store you may find the cookie is too large and doesn't get passed through correctly. Increasing the proxy_buffer_size in nginx or implementing the [redis session storage](sessions.md#redis-storage) should resolve this.
+
 ### Facebook Auth Provider
 
 1.  Create a new FB App from <https://developers.facebook.com/>
diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go
index 3b8d0da4..5efa6f20 100644
--- a/pkg/apis/options/providers.go
+++ b/pkg/apis/options/providers.go
@@ -21,6 +21,8 @@ type Provider struct {
 	KeycloakConfig KeycloakOptions `json:"keycloakConfig,omitempty"`
 	// AzureConfig holds all configurations for Azure provider.
 	AzureConfig AzureOptions `json:"azureConfig,omitempty"`
+	// ADFSConfig holds all configurations for ADFS provider.
+	ADFSConfig ADFSOptions `json:"ADFSConfig,omitempty"`
 	// BitbucketConfig holds all configurations for Bitbucket provider.
 	BitbucketConfig BitbucketOptions `json:"bitbucketConfig,omitempty"`
 	// GitHubConfig holds all configurations for GitHubC provider.
@@ -55,7 +57,7 @@ type Provider struct {
 	RedeemURL string `json:"redeemURL,omitempty"`
 	// ProfileURL is the profile access endpoint
 	ProfileURL string `json:"profileURL,omitempty"`
-	// ProtectedResource is the resource that is protected (Azure AD only)
+	// ProtectedResource is the resource that is protected (Azure AD and ADFS only)
 	ProtectedResource string `json:"resource,omitempty"`
 	// ValidateURL is the access token validation endpoint
 	ValidateURL string `json:"validateURL,omitempty"`
@@ -84,6 +86,12 @@ type AzureOptions struct {
 	Tenant string `json:"tenant,omitempty"`
 }
 
+type ADFSOptions struct {
+	// Skip adding the scope parameter in login request
+	// Default value is 'false'
+	SkipScope bool `json:"skipScope,omitempty"`
+}
+
 type BitbucketOptions struct {
 	// Team sets restrict logins to members of this team
 	Team string `json:"team,omitempty"`
diff --git a/pkg/validation/options.go b/pkg/validation/options.go
index 2e5b884d..73f14b72 100644
--- a/pkg/validation/options.go
+++ b/pkg/validation/options.go
@@ -236,6 +236,8 @@ func parseProviderInfo(o *options.Options, msgs []string) []string {
 	switch p := o.GetProvider().(type) {
 	case *providers.AzureProvider:
 		p.Configure(o.Providers[0].AzureConfig.Tenant)
+	case *providers.ADFSProvider:
+		p.Configure(o.Providers[0].ADFSConfig.SkipScope)
 	case *providers.GitHubProvider:
 		p.SetOrgTeam(o.Providers[0].GitHubConfig.Org, o.Providers[0].GitHubConfig.Team)
 		p.SetRepo(o.Providers[0].GitHubConfig.Repo, o.Providers[0].GitHubConfig.Token)
diff --git a/providers/adfs.go b/providers/adfs.go
new file mode 100644
index 00000000..c626bd09
--- /dev/null
+++ b/providers/adfs.go
@@ -0,0 +1,100 @@
+package providers
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/url"
+	"strings"
+
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
+)
+
+// ADFSProvider represents an ADFS based Identity Provider
+type ADFSProvider struct {
+	*OIDCProvider
+	SkipScope bool
+}
+
+var _ Provider = (*ADFSProvider)(nil)
+
+const (
+	ADFSProviderName = "ADFS"
+	ADFSDefaultScope = "openid email profile"
+	ADFSSkipScope    = false
+)
+
+// NewADFSProvider initiates a new ADFSProvider
+func NewADFSProvider(p *ProviderData) *ADFSProvider {
+
+	p.setProviderDefaults(providerDefaults{
+		name:  ADFSProviderName,
+		scope: ADFSDefaultScope,
+	})
+
+	if p.ProtectedResource != nil && p.ProtectedResource.String() != "" {
+		resource := p.ProtectedResource.String()
+		if !strings.HasSuffix(resource, "/") {
+			resource += "/"
+		}
+
+		if p.Scope != "" && !strings.HasPrefix(p.Scope, resource) {
+			p.Scope = resource + p.Scope
+		}
+	}
+
+	return &ADFSProvider{
+		OIDCProvider: &OIDCProvider{
+			ProviderData: p,
+			SkipNonce:    true,
+		},
+		SkipScope: ADFSSkipScope,
+	}
+}
+
+// Configure defaults the ADFSProvider configuration options
+func (p *ADFSProvider) Configure(skipScope bool) {
+	p.SkipScope = skipScope
+}
+
+// GetLoginURL Override to double encode the state parameter. If not query params are lost
+// More info here: https://docs.microsoft.com/en-us/powerapps/maker/portals/configure/configure-saml2-settings
+func (p *ADFSProvider) GetLoginURL(redirectURI, state, nonce string) string {
+	extraParams := url.Values{}
+	if !p.SkipNonce {
+		extraParams.Add("nonce", nonce)
+	}
+	loginURL := makeLoginURL(p.Data(), redirectURI, url.QueryEscape(state), extraParams)
+	if p.SkipScope {
+		q := loginURL.Query()
+		q.Del("scope")
+		loginURL.RawQuery = q.Encode()
+	}
+	return loginURL.String()
+}
+
+// EnrichSession to add email
+func (p *ADFSProvider) EnrichSession(ctx context.Context, s *sessions.SessionState) error {
+	if s.Email != "" {
+		return nil
+	}
+
+	idToken, err := p.Verifier.Verify(ctx, s.IDToken)
+	if err != nil {
+		return err
+	}
+
+	p.EmailClaim = "upn"
+	c, err := p.getClaims(idToken)
+
+	if err != nil {
+		return fmt.Errorf("couldn't extract claims from id_token (%v)", err)
+	}
+	s.Email = c.Email
+
+	if s.Email == "" {
+		err = errors.New("email not set")
+	}
+
+	return err
+}
diff --git a/providers/providers.go b/providers/providers.go
index 498d1db0..0340c420 100644
--- a/providers/providers.go
+++ b/providers/providers.go
@@ -33,6 +33,8 @@ func New(provider string, p *ProviderData) Provider {
 		return NewKeycloakProvider(p)
 	case "azure":
 		return NewAzureProvider(p)
+	case "adfs":
+		return NewADFSProvider(p)
 	case "gitlab":
 		return NewGitLabProvider(p)
 	case "oidc":


Test patch:
diff --git a/providers/adfs_test.go b/providers/adfs_test.go
new file mode 100644
index 00000000..4c8764bd
--- /dev/null
+++ b/providers/adfs_test.go
@@ -0,0 +1,205 @@
+package providers
+
+import (
+	"context"
+	"encoding/base64"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+
+	"github.com/coreos/go-oidc"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+)
+
+type fakeADFSJwks struct{}
+
+func (fakeADFSJwks) VerifySignature(_ context.Context, jwt string) (payload []byte, err error) {
+	decodeString, err := base64.RawURLEncoding.DecodeString(strings.Split(jwt, ".")[1])
+	if err != nil {
+		return nil, err
+	}
+	return decodeString, nil
+}
+
+func testADFSProvider(hostname string) *ADFSProvider {
+
+	o := oidc.NewVerifier(
+		"https://issuer.example.com",
+		fakeADFSJwks{},
+		&oidc.Config{ClientID: "https://test.myapp.com"},
+	)
+
+	p := NewADFSProvider(&ProviderData{
+		ProviderName: "",
+		LoginURL:     &url.URL{},
+		RedeemURL:    &url.URL{},
+		ProfileURL:   &url.URL{},
+		ValidateURL:  &url.URL{},
+		Scope:        "",
+		Verifier:     o,
+	})
+
+	if hostname != "" {
+		updateURL(p.Data().LoginURL, hostname)
+		updateURL(p.Data().RedeemURL, hostname)
+		updateURL(p.Data().ProfileURL, hostname)
+		updateURL(p.Data().ValidateURL, hostname)
+	}
+
+	return p
+}
+
+func testADFSBackend() *httptest.Server {
+
+	authResponse := `
+		{
+			"access_token": "my_access_token",
+			"id_token": "my_id_token",
+			"refresh_token": "my_refresh_token" 
+		 }
+	`
+	userInfo := `
+		{
+			"email": "samiracho@email.com"
+		}
+	`
+
+	refreshResponse := `{ "access_token": "new_some_access_token", "refresh_token": "new_some_refresh_token", "expires_in": "32693148245", "id_token": "new_some_id_token" }`
+
+	authHeader := "Bearer adfs_access_token"
+
+	return httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			switch r.URL.Path {
+			case "/adfs/oauth2/authorize":
+				w.WriteHeader(200)
+				w.Write([]byte(authResponse))
+			case "/adfs/oauth2/refresh":
+				w.WriteHeader(200)
+				w.Write([]byte(refreshResponse))
+			case "/adfs/oauth2/userinfo":
+				if r.Header["Authorization"][0] == authHeader {
+					w.WriteHeader(200)
+					w.Write([]byte(userInfo))
+				} else {
+					w.WriteHeader(401)
+				}
+			default:
+				w.WriteHeader(200)
+			}
+		}))
+}
+
+var _ = Describe("ADFS Provider Tests", func() {
+	var p *ADFSProvider
+	var b *httptest.Server
+
+	BeforeEach(func() {
+		b = testADFSBackend()
+
+		bURL, err := url.Parse(b.URL)
+		Expect(err).To(BeNil())
+
+		p = testADFSProvider(bURL.Host)
+	})
+
+	AfterEach(func() {
+		b.Close()
+	})
+
+	Context("New Provider Init", func() {
+		It("uses defaults", func() {
+			providerData := NewADFSProvider(&ProviderData{}).Data()
+			Expect(providerData.ProviderName).To(Equal("ADFS"))
+			Expect(providerData.Scope).To(Equal("openid email profile"))
+		})
+	})
+
+	Context("with bad token", func() {
+		It("should trigger an error", func() {
+			session := &sessions.SessionState{AccessToken: "unexpected_adfs_access_token", IDToken: "malformed_token"}
+			err := p.EnrichSession(context.Background(), session)
+			Expect(err).NotTo(BeNil())
+		})
+	})
+
+	Context("with valid token", func() {
+		It("should not throw an error", func() {
+			p.EmailClaim = "email"
+			rawIDToken, _ := newSignedTestIDToken(defaultIDToken)
+			idToken, err := p.Verifier.Verify(context.Background(), rawIDToken)
+			Expect(err).To(BeNil())
+			session, err := p.buildSessionFromClaims(idToken)
+			session.IDToken = rawIDToken
+			Expect(err).To(BeNil())
+			err = p.EnrichSession(context.Background(), session)
+			Expect(session.Email).To(Equal("janed@me.com"))
+			Expect(err).To(BeNil())
+		})
+	})
+
+	Context("with skipScope enabled", func() {
+		It("should not include parameter scope", func() {
+			resource, _ := url.Parse("http://example.com")
+			p := NewADFSProvider(&ProviderData{
+				ProtectedResource: resource,
+				Scope:             "",
+			})
+			p.SkipScope = true
+
+			result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "")
+			Expect(result).NotTo(ContainSubstring("scope="))
+		})
+	})
+
+	Context("With resource parameter", func() {
+		type scopeTableInput struct {
+			resource      string
+			scope         string
+			expectedScope string
+		}
+
+		DescribeTable("should return expected results",
+			func(in scopeTableInput) {
+				resource, _ := url.Parse(in.resource)
+				p := NewADFSProvider(&ProviderData{
+					ProtectedResource: resource,
+					Scope:             in.scope,
+				})
+
+				Expect(p.Data().Scope).To(Equal(in.expectedScope))
+				result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "")
+				Expect(result).To(ContainSubstring("scope=" + url.QueryEscape(in.expectedScope)))
+			},
+			Entry("should add slash", scopeTableInput{
+				resource:      "http://resource.com",
+				scope:         "openid",
+				expectedScope: "http://resource.com/openid",
+			}),
+			Entry("shouldn't add extra slash", scopeTableInput{
+				resource:      "http://resource.com/",
+				scope:         "openid",
+				expectedScope: "http://resource.com/openid",
+			}),
+			Entry("should add default scopes with resource", scopeTableInput{
+				resource:      "http://resource.com/",
+				scope:         "",
+				expectedScope: "http://resource.com/openid email profile",
+			}),
+			Entry("should add default scopes", scopeTableInput{
+				resource:      "",
+				scope:         "",
+				expectedScope: "openid email profile",
+			}),
+			Entry("shouldn't add resource if already in scopes", scopeTableInput{
+				resource:      "http://resource.com",
+				scope:         "http://resource.com/openid",
+				expectedScope: "http://resource.com/openid",
+			}),
+		)
+	})
+})