Repository: oauth2-proxy/oauth2-proxy
Pull request #1509 (2022-01-07 20:38:27)

Add ValidateSession function to LoginGovProvder to include Auth Header
## Description

Add ValidateSession function to LoginGovProvider to include Authorization header using access_token

## Motivation and Context
This recent bug fix is causing Login.gov oauth to fail "[#1433](https://github.com/oauth2-proxy/oauth2-proxy/pull/1433) Let authentication fail when session validation fails (@stippi2)"
We need to be able to update as this and future versions to resolve new CVE's.

## How Has This Been Tested?

I've tested this locally running the module and in our K8s test cluster to ensure it to work.

## Checklist:

- [x] My change requires a change to the documentation or CHANGELOG.
- [ ] I have updated the documentation/CHANGELOG accordingly.
- [x ] I have created a feature (non-master) branch for my PR.


JoelSpeed: Seems reasonable, would you be able to add a changelog entry and then we can get this merged
pksheldon4: > Seems reasonable, would you be able to add a changelog entry and then we can get this merged

Done. Newish to this so please let me know if you need anything else from me.
pksheldon4: Just checking in to see if this is waiting on anything from me ? Or what the cadence is for new releases ?
pksheldon4: > Sorry for the delay, very busy at the moment so not getting much time to look at the project

I understand. Just wanted to make sure my lack of understanding wasn't delaying things.  I don't see any errors in the build, except for the Failed at the very end. I'm not sure how to proceed.
JoelSpeed: One of the tests looks like it'll need an update
https://github.com/oauth2-proxy/oauth2-proxy/runs/5054354303?check_suite_focus=true#step:8:1352
pksheldon4: > 



> One of the tests looks like it'll need an update https://github.com/oauth2-proxy/oauth2-proxy/runs/5054354303?check_suite_focus=true#step:8:1352

I'm trying to understand the error as it's not obvious from the logs.  I also tried running the tests on the latest code https://github.com/oauth2-proxy/oauth2-proxy/ using `make test` and I get the same error.
JoelSpeed: The failure, just to confirm is:
```
=== RUN   TestNewLoginGovProvider
    logingov_test.go:78: 
        Expected
            <string>: https://secure.login.gov/api/openid_connect/userinfo
        to equal
            <string>: 
--- FAIL: TestNewLoginGovProvider (0.00s)
```
I'm not seeing this error when I run against the master branch. The test is related to how the logingov provider is configured so it seems like it could be linked to this PR
JoelSpeed: Seems to me like we need to update this test case with the new validate URL
https://github.com/oauth2-proxy/oauth2-proxy/blob/4f5efd4074098223aeee244b963dafa9cfd4924b/providers/logingov_test.go#L78
pksheldon4: > The failure, just to confirm is:
> 
> ```
> === RUN   TestNewLoginGovProvider
>     logingov_test.go:78: 
>         Expected
>             <string>: https://secure.login.gov/api/openid_connect/userinfo
>         to equal
>             <string>: 
> --- FAIL: TestNewLoginGovProvider (0.00s)
> ```
> 
> I'm not seeing this error when I run against the master branch. The test is related to how the logingov provider is configured so it seems like it could be linked to this PR

Yes, this is definitely on me. Still getting used to how to test this project.  If I just run `make test` or even `./.github/workflows/test.sh` that test doesn't seem to run.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f75520da..4b8cf14a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@
 
 - [#1489](https://github.com/oauth2-proxy/oauth2-proxy/pull/1489) Fix Docker Buildx push to include build version (@JoelSpeed)
 - [#1477](https://github.com/oauth2-proxy/oauth2-proxy/pull/1477) Remove provider documentation for `Microsoft Azure AD` (@omBratteng)
+- [#1509](https://github.com/oauth2-proxy/oauth2-proxy/pull/1509) Update LoginGovProvider ValidateSession to pass access_token in Header (@pksheldon4)
 
 # V7.2.1
 
diff --git a/providers/logingov.go b/providers/logingov.go
index 80b5e7ad..103fdf11 100644
--- a/providers/logingov.go
+++ b/providers/logingov.go
@@ -84,7 +84,7 @@ func NewLoginGovProvider(p *ProviderData) *LoginGovProvider {
 		loginURL:    loginGovDefaultLoginURL,
 		redeemURL:   loginGovDefaultRedeemURL,
 		profileURL:  loginGovDefaultProfileURL,
-		validateURL: nil,
+		validateURL: loginGovDefaultProfileURL,
 		scope:       loginGovDefaultScope,
 	})
 	return &LoginGovProvider{
@@ -237,3 +237,8 @@ func (p *LoginGovProvider) GetLoginURL(redirectURI, state, _ string) string {
 	a := makeLoginURL(p.ProviderData, redirectURI, state, extraParams)
 	return a.String()
 }
+
+// ValidateSession validates the AccessToken
+func (p *LoginGovProvider) ValidateSession(ctx context.Context, s *sessions.SessionState) bool {
+	return validateToken(ctx, p, s.AccessToken, makeOIDCHeader(s.AccessToken))
+}


Test patch:
diff --git a/providers/logingov_test.go b/providers/logingov_test.go
index 3ed0f520..db30262e 100644
--- a/providers/logingov_test.go
+++ b/providers/logingov_test.go
@@ -75,7 +75,7 @@ func TestNewLoginGovProvider(t *testing.T) {
 	g.Expect(providerData.LoginURL.String()).To(Equal("https://secure.login.gov/openid_connect/authorize"))
 	g.Expect(providerData.RedeemURL.String()).To(Equal("https://secure.login.gov/api/openid_connect/token"))
 	g.Expect(providerData.ProfileURL.String()).To(Equal("https://secure.login.gov/api/openid_connect/userinfo"))
-	g.Expect(providerData.ValidateURL.String()).To(Equal(""))
+	g.Expect(providerData.ValidateURL.String()).To(Equal("https://secure.login.gov/api/openid_connect/userinfo"))
 	g.Expect(providerData.Scope).To(Equal("email openid"))
 }