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

Add IDToken for Azure provider #258

Merged
merged 12 commits into from
Oct 17, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
- [#227](https://github.com/pusher/oauth2_proxy/pull/227) Add Keycloak provider (@Ofinka)
- [#273](https://github.com/pusher/oauth2_proxy/pull/273) Support Go 1.13 (@dio)
- [#275](https://github.com/pusher/oauth2_proxy/pull/275) docker: build from debian buster (@syscll)
- [#258](https://github.com/pusher/oauth2_proxy/pull/258) Add IDToken for Azure provider
- This PR adds the IDToken into the session for the Azure provider allowing requests to a backend to be identified as a specific user. As a consequence, if you are using a cookie to store the session the cookie will now exceed the 4kb size limit and be split into multiple cookies. This can cause problems when using nginx as a proxy, resulting in no cookie being passed at all. Either increase the proxy_buffer_size in nginx or implement the redis session storage (see https://pusher.github.io/oauth2_proxy/configuration#redis-storage)
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this note is definitely relevant but maybe the changelog is the wrong place? WDYT @JoelSpeed @steakunderscore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's also been added to the Azure section of the Authentication documentation in this pull request. Obviously though this is very much worth highlighting to people as I'd imagine it will break a lot of configurations so if there is some where else we can put it to draw peoples attention to it let me know.

Copy link
Member

Choose a reason for hiding this comment

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

I am happy to have a note like this in the changelog, it's important that people are aware of changes like this that may break their setups.

It's the sort of thing I'll copy out to highlight it in the release highlights when I cut the next release

Copy link
Member

@loshz loshz Oct 17, 2019

Choose a reason for hiding this comment

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

Totally happy for it to stay in the changelog 🙂


# v4.0.0

Expand Down
2 changes: 2 additions & 0 deletions docs/2_auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ Note: The user is checked against the group members list on initial authenticati
--client-secret=<value from step 6>
```

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](configuration#redis-storage) should resolve this.

### Facebook Auth Provider

1. Create a new FB App from <https://developers.facebook.com/>
Expand Down
65 changes: 65 additions & 0 deletions providers/azure.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package providers

import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"net/http"
"net/url"
"time"

"github.com/bitly/go-simplejson"
"github.com/pusher/oauth2_proxy/pkg/apis/sessions"
Expand Down Expand Up @@ -65,6 +69,67 @@ func (p *AzureProvider) Configure(tenant string) {
}
}

func (p *AzureProvider) Redeem(redirectURL, code string) (s *sessions.SessionState, err error) {
if code == "" {
err = errors.New("missing code")
return
}

params := url.Values{}
params.Add("redirect_uri", redirectURL)
params.Add("client_id", p.ClientID)
params.Add("client_secret", p.ClientSecret)
params.Add("code", code)
params.Add("grant_type", "authorization_code")
if p.ProtectedResource != nil && p.ProtectedResource.String() != "" {
params.Add("resource", p.ProtectedResource.String())
}

var req *http.Request
req, err = http.NewRequest("POST", p.RedeemURL.String(), bytes.NewBufferString(params.Encode()))
if err != nil {
return
}
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

var resp *http.Response
resp, err = http.DefaultClient.Do(req)
if err != nil {
return nil, err
}
var body []byte
body, err = ioutil.ReadAll(resp.Body)
resp.Body.Close()
if err != nil {
return
}

if resp.StatusCode != 200 {
err = fmt.Errorf("got %d from %q %s", resp.StatusCode, p.RedeemURL.String(), body)
return
}

var jsonResponse struct {
AccessToken string `json:"access_token"`
RefreshToken string `json:"refresh_token"`
ExpiresOn int64 `json:"expires_on,string"`
IDToken string `json:"id_token"`
}
err = json.Unmarshal(body, &jsonResponse)
if err != nil {
return
}

s = &sessions.SessionState{
AccessToken: jsonResponse.AccessToken,
IDToken: jsonResponse.IDToken,
CreatedAt: time.Now(),
ExpiresOn: time.Unix(jsonResponse.ExpiresOn, 0),
RefreshToken: jsonResponse.RefreshToken,
}
return
}

func getAzureHeader(accessToken string) http.Header {
header := make(http.Header)
header.Set("Authorization", fmt.Sprintf("Bearer %s", accessToken))
Expand Down
23 changes: 22 additions & 1 deletion providers/azure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http/httptest"
"net/url"
"testing"
"time"

"github.com/pusher/oauth2_proxy/pkg/apis/sessions"
"github.com/stretchr/testify/assert"
Expand All @@ -20,6 +21,7 @@ func testAzureProvider(hostname string) *AzureProvider {
ValidateURL: &url.URL{},
ProtectedResource: &url.URL{},
Scope: ""})

if hostname != "" {
updateURL(p.Data().LoginURL, hostname)
updateURL(p.Data().RedeemURL, hostname)
Expand Down Expand Up @@ -111,8 +113,11 @@ func testAzureBackend(payload string) *httptest.Server {

return httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != path || r.URL.RawQuery != query {
if (r.URL.Path != path || r.URL.RawQuery != query) && r.Method != "POST" {
w.WriteHeader(404)
} else if r.Method == "POST" && r.Body != nil {
w.WriteHeader(200)
w.Write([]byte(payload))
} else if r.Header.Get("Authorization") != "Bearer imaginary_access_token" {
w.WriteHeader(403)
} else {
Expand Down Expand Up @@ -199,3 +204,19 @@ func TestAzureProviderGetEmailAddressIncorrectOtherMails(t *testing.T) {
assert.Equal(t, "type assertion to string failed", err.Error())
assert.Equal(t, "", email)
}

func TestAzureProviderRedeemReturnsIdToken(t *testing.T) {
b := testAzureBackend(`{ "id_token": "testtoken1234", "expires_on": "1136239445", "refresh_token": "refresh1234" }`)
defer b.Close()
timestamp, err := time.Parse(time.RFC3339, "2006-01-02T22:04:05Z")
assert.Equal(t, nil, err)

bURL, _ := url.Parse(b.URL)
p := testAzureProvider(bURL.Host)
p.Data().RedeemURL.Path = "/common/oauth2/token"
s, err := p.Redeem("https://localhost", "1234")
assert.Equal(t, nil, err)
assert.Equal(t, "testtoken1234", s.IDToken)
assert.Equal(t, timestamp, s.ExpiresOn.UTC())
assert.Equal(t, "refresh1234", s.RefreshToken)
}