git.iko.soy/typeable/oauth2-proxy
on master help sign in
Github provider: use login as user

- Save both user and email in session state: Encoding/decoding methods save both email and user field in session state, for use cases when User is not derived from email's local-parth, like for GitHub provider. For retrocompatibility, if no user is obtained by the provider, (e.g. User is an empty string) the encoding/decoding methods fall back to the previous behavior and use the email's local-part Updated also related tests and added two more tests to show behavior when session contains a non-empty user value. - Added first basic GitHub provider tests - Added GetUserName method to Provider interface The new GetUserName method is intended to return the User value when this is not the email's local-part. Added also the default implementation to provider_default.go - Added call to GetUserName in redeemCode the new GetUserName method is used in redeemCode to get SessionState User value. For backward compatibility, if GetUserName error is "not implemented", the error is ignored. - Added GetUserName method and tests to github provider.

commit731fa9f8e00b294ff1bf4a687e63d2ecdd9a4a50 authorCarlo Lobrano <c.lobrano@gmail.com> dateTue 26 Sep 2017, 23:31 (9 years ago) parent6ddbb2c
7 files changed+313−43
compare with master
unifiedsplit
oauthproxy.go+7 providers/github.go+45−2 providers/github_test.go+146 providers/provider_default.go+5 providers/providers.go+1 providers/session_state.go+40−36 providers/session_state_test.go+69−5
oauthproxy.go+7
@@ -244,6 +244,13 @@ func (p *OAuthProxy) redeemCode(host, code string) (s *providers.SessionState, e
244244 if s.Email == "" {
245245 s.Email, err = p.provider.GetEmailAddress(s)
246246 }
247+
248+ if s.User == "" {
249+ s.User, err = p.provider.GetUserName(s)
250+ if err != nil && err.Error() == "not implemented" {
251+ err = nil
252+ }
253+ }
247254 return
248255}
249256
providers/github.go+45−2
@@ -218,10 +218,10 @@ func (p *GitHubProvider) GetEmailAddress(s *SessionState) (string, error) {
218218 if resp.StatusCode != 200 {
219219 return "", fmt.Errorf("got %d from %q %s",
220220 resp.StatusCode, endpoint.String(), body)
221− } else {
222− log.Printf("got %d from %q %s", resp.StatusCode, endpoint.String(), body)
223221 }
224222
223+ log.Printf("got %d from %q %s", resp.StatusCode, endpoint.String(), body)
224+
225225 if err := json.Unmarshal(body, &emails); err != nil {
226226 return "", fmt.Errorf("%s unmarshaling %s", err, body)
227227 }
@@ -234,3 +234,46 @@ func (p *GitHubProvider) GetEmailAddress(s *SessionState) (string, error) {
234234
235235 return "", nil
236236}
237+
238+func (p *GitHubProvider) GetUserName(s *SessionState) (string, error) {
239+ var user struct {
240+ Login string `json:"login"`
241+ Email string `json:"email"`
242+ }
243+
244+ endpoint := &url.URL{
245+ Scheme: p.ValidateURL.Scheme,
246+ Host: p.ValidateURL.Host,
247+ Path: path.Join(p.ValidateURL.Path, "/user"),
248+ }
249+
250+ req, err := http.NewRequest("GET", endpoint.String(), nil)
251+ if err != nil {
252+ return "", fmt.Errorf("could not create new GET request: %v", err)
253+ }
254+
255+ req.Header.Set("Authorization", fmt.Sprintf("token %s", s.AccessToken))
256+ resp, err := http.DefaultClient.Do(req)
257+ if err != nil {
258+ return "", err
259+ }
260+
261+ body, err := ioutil.ReadAll(resp.Body)
262+ defer resp.Body.Close()
263+ if err != nil {
264+ return "", err
265+ }
266+
267+ if resp.StatusCode != 200 {
268+ return "", fmt.Errorf("got %d from %q %s",
269+ resp.StatusCode, endpoint.String(), body)
270+ }
271+
272+ log.Printf("got %d from %q %s", resp.StatusCode, endpoint.String(), body)
273+
274+ if err := json.Unmarshal(body, &user); err != nil {
275+ return "", fmt.Errorf("%s unmarshaling %s", err, body)
276+ }
277+
278+ return user.Login, nil
279+}
providers/github_test.gonew file+146
@@ -0,0 +1,146 @@
1+package providers
2+
3+import (
4+ "net/http"
5+ "net/http/httptest"
6+ "net/url"
7+ "testing"
8+
9+ "github.com/stretchr/testify/assert"
10+)
11+
12+func testGitHubProvider(hostname string) *GitHubProvider {
13+ p := NewGitHubProvider(
14+ &ProviderData{
15+ ProviderName: "",
16+ LoginURL: &url.URL{},
17+ RedeemURL: &url.URL{},
18+ ProfileURL: &url.URL{},
19+ ValidateURL: &url.URL{},
20+ Scope: ""})
21+ if hostname != "" {
22+ updateURL(p.Data().LoginURL, hostname)
23+ updateURL(p.Data().RedeemURL, hostname)
24+ updateURL(p.Data().ProfileURL, hostname)
25+ updateURL(p.Data().ValidateURL, hostname)
26+ }
27+ return p
28+}
29+
30+func testGitHubBackend(payload string) *httptest.Server {
31+ pathToQueryMap := map[string]string{
32+ "/user": "",
33+ "/user/emails": "",
34+ }
35+
36+ return httptest.NewServer(http.HandlerFunc(
37+ func(w http.ResponseWriter, r *http.Request) {
38+ url := r.URL
39+ query, ok := pathToQueryMap[url.Path]
40+ if !ok {
41+ w.WriteHeader(404)
42+ } else if url.RawQuery != query {
43+ w.WriteHeader(404)
44+ } else {
45+ w.WriteHeader(200)
46+ w.Write([]byte(payload))
47+ }
48+ }))
49+}
50+
51+func TestGitHubProviderDefaults(t *testing.T) {
52+ p := testGitHubProvider("")
53+ assert.NotEqual(t, nil, p)
54+ assert.Equal(t, "GitHub", p.Data().ProviderName)
55+ assert.Equal(t, "https://github.com/login/oauth/authorize",
56+ p.Data().LoginURL.String())
57+ assert.Equal(t, "https://github.com/login/oauth/access_token",
58+ p.Data().RedeemURL.String())
59+ assert.Equal(t, "https://api.github.com/",
60+ p.Data().ValidateURL.String())
61+ assert.Equal(t, "user:email", p.Data().Scope)
62+}
63+
64+func TestGitHubProviderOverrides(t *testing.T) {
65+ p := NewGitHubProvider(
66+ &ProviderData{
67+ LoginURL: &url.URL{
68+ Scheme: "https",
69+ Host: "example.com",
70+ Path: "/login/oauth/authorize"},
71+ RedeemURL: &url.URL{
72+ Scheme: "https",
73+ Host: "example.com",
74+ Path: "/login/oauth/access_token"},
75+ ValidateURL: &url.URL{
76+ Scheme: "https",
77+ Host: "api.example.com",
78+ Path: "/"},
79+ Scope: "profile"})
80+ assert.NotEqual(t, nil, p)
81+ assert.Equal(t, "GitHub", p.Data().ProviderName)
82+ assert.Equal(t, "https://example.com/login/oauth/authorize",
83+ p.Data().LoginURL.String())
84+ assert.Equal(t, "https://example.com/login/oauth/access_token",
85+ p.Data().RedeemURL.String())
86+ assert.Equal(t, "https://api.example.com/",
87+ p.Data().ValidateURL.String())
88+ assert.Equal(t, "profile", p.Data().Scope)
89+}
90+
91+func TestGitHubProviderGetEmailAddress(t *testing.T) {
92+ b := testGitHubBackend(`[ {"email": "michael.bland@gsa.gov", "primary": true} ]`)
93+ defer b.Close()
94+
95+ bURL, _ := url.Parse(b.URL)
96+ p := testGitHubProvider(bURL.Host)
97+
98+ session := &SessionState{AccessToken: "imaginary_access_token"}
99+ email, err := p.GetEmailAddress(session)
100+ assert.Equal(t, nil, err)
101+ assert.Equal(t, "michael.bland@gsa.gov", email)
102+}
103+
104+// Note that trying to trigger the "failed building request" case is not
105+// practical, since the only way it can fail is if the URL fails to parse.
106+func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) {
107+ b := testGitHubBackend("unused payload")
108+ defer b.Close()
109+
110+ bURL, _ := url.Parse(b.URL)
111+ p := testGitHubProvider(bURL.Host)
112+
113+ // We'll trigger a request failure by using an unexpected access
114+ // token. Alternatively, we could allow the parsing of the payload as
115+ // JSON to fail.
116+ session := &SessionState{AccessToken: "unexpected_access_token"}
117+ email, err := p.GetEmailAddress(session)
118+ assert.NotEqual(t, nil, err)
119+ assert.Equal(t, "", email)
120+}
121+
122+func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) {
123+ b := testGitHubBackend("{\"foo\": \"bar\"}")
124+ defer b.Close()
125+
126+ bURL, _ := url.Parse(b.URL)
127+ p := testGitHubProvider(bURL.Host)
128+
129+ session := &SessionState{AccessToken: "imaginary_access_token"}
130+ email, err := p.GetEmailAddress(session)
131+ assert.NotEqual(t, nil, err)
132+ assert.Equal(t, "", email)
133+}
134+
135+func TestGitHubProviderGetUserName(t *testing.T) {
136+ b := testGitHubBackend(`{"email": "michael.bland@gsa.gov", "login": "mbland"}`)
137+ defer b.Close()
138+
139+ bURL, _ := url.Parse(b.URL)
140+ p := testGitHubProvider(bURL.Host)
141+
142+ session := &SessionState{AccessToken: "imaginary_access_token"}
143+ email, err := p.GetUserName(session)
144+ assert.Equal(t, nil, err)
145+ assert.Equal(t, "mbland", email)
146+}
providers/provider_default.go+5
@@ -106,6 +106,11 @@ func (p *ProviderData) GetEmailAddress(s *SessionState) (string, error) {
106106 return "", errors.New("not implemented")
107107}
108108
109+// GetUserName returns the Account username
110+func (p *ProviderData) GetUserName(s *SessionState) (string, error) {
111+ return "", errors.New("not implemented")
112+}
113+
109114// ValidateGroup validates that the provided email exists in the configured provider
110115// email group(s).
111116func (p *ProviderData) ValidateGroup(email string) bool {
providers/providers.go+1
@@ -7,6 +7,7 @@ import (
77type Provider interface {
88 Data() *ProviderData
99 GetEmailAddress(*SessionState) (string, error)
10+ GetUserName(*SessionState) (string, error)
1011 Redeem(string, string) (*SessionState, error)
1112 ValidateGroup(string) bool
1213 ValidateSessionState(*SessionState) bool
providers/session_state.go+40−36
@@ -25,7 +25,7 @@ func (s *SessionState) IsExpired() bool {
2525}
2626
2727func (s *SessionState) String() string {
28− o := fmt.Sprintf("Session{%s", s.userOrEmail())
28+ o := fmt.Sprintf("Session{%s", s.accountInfo())
2929 if s.AccessToken != "" {
3030 o += " token:true"
3131 }
@@ -40,17 +40,13 @@ func (s *SessionState) String() string {
4040
4141func (s *SessionState) EncodeSessionState(c *cookie.Cipher) (string, error) {
4242 if c == nil || s.AccessToken == "" {
43− return s.userOrEmail(), nil
43+ return s.accountInfo(), nil
4444 }
4545 return s.EncryptedString(c)
4646}
4747
48−func (s *SessionState) userOrEmail() string {
49− u := s.User
50− if s.Email != "" {
51− u = s.Email
52− }
53− return u
48+func (s *SessionState) accountInfo() string {
49+ return fmt.Sprintf("email:%s user:%s", s.Email, s.User)
5450}
5551
5652func (s *SessionState) EncryptedString(c *cookie.Cipher) (string, error) {
@@ -60,56 +56,64 @@ func (s *SessionState) EncryptedString(c *cookie.Cipher) (string, error) {
6056 }
6157 a := s.AccessToken
6258 if a != "" {
63− a, err = c.Encrypt(a)
64− if err != nil {
59+ if a, err = c.Encrypt(a); err != nil {
6560 return "", err
6661 }
6762 }
6863 r := s.RefreshToken
6964 if r != "" {
70− r, err = c.Encrypt(r)
71− if err != nil {
65+ if r, err = c.Encrypt(r); err != nil {
7266 return "", err
7367 }
7468 }
75− return fmt.Sprintf("%s|%s|%d|%s", s.userOrEmail(), a, s.ExpiresOn.Unix(), r), nil
69+ return fmt.Sprintf("%s|%s|%d|%s", s.accountInfo(), a, s.ExpiresOn.Unix(), r), nil
70+}
71+
72+func decodeSessionStatePlain(v string) (s *SessionState, err error) {
73+ chunks := strings.Split(v, " ")
74+ if len(chunks) != 2 {
75+ return nil, fmt.Errorf("could not decode session state: expected 2 chunks got %d", len(chunks))
76+ }
77+
78+ email := strings.TrimPrefix(chunks[0], "email:")
79+ user := strings.TrimPrefix(chunks[1], "user:")
80+ if user == "" {
81+ user = strings.Split(email, "@")[0]
82+ }
83+
84+ return &SessionState{User: user, Email: email}, nil
7685}
7786
7887func DecodeSessionState(v string, c *cookie.Cipher) (s *SessionState, err error) {
79− chunks := strings.Split(v, "|")
80− if len(chunks) == 1 {
81− if strings.Contains(chunks[0], "@") {
82− u := strings.Split(v, "@")[0]
83− return &SessionState{Email: v, User: u}, nil
84− }
85− return &SessionState{User: v}, nil
88+ if c == nil {
89+ return decodeSessionStatePlain(v)
8690 }
8791
92+ chunks := strings.Split(v, "|")
8893 if len(chunks) != 4 {
8994 err = fmt.Errorf("invalid number of fields (got %d expected 4)", len(chunks))
9095 return
9196 }
9297
93− s = &SessionState{}
94− if c != nil && chunks[1] != "" {
95− s.AccessToken, err = c.Decrypt(chunks[1])
96− if err != nil {
98+ sessionState, err := decodeSessionStatePlain(chunks[0])
99+ if err != nil {
100+ return nil, err
101+ }
102+
103+ if chunks[1] != "" {
104+ if sessionState.AccessToken, err = c.Decrypt(chunks[1]); err != nil {
97105 return nil, err
98106 }
99107 }
100− if c != nil && chunks[3] != "" {
101− s.RefreshToken, err = c.Decrypt(chunks[3])
102− if err != nil {
108+
109+ ts, _ := strconv.Atoi(chunks[2])
110+ sessionState.ExpiresOn = time.Unix(int64(ts), 0)
111+
112+ if chunks[3] != "" {
113+ if sessionState.RefreshToken, err = c.Decrypt(chunks[3]); err != nil {
103114 return nil, err
104115 }
105116 }
106− if u := chunks[0]; strings.Contains(u, "@") {
107− s.Email = u
108− s.User = strings.Split(u, "@")[0]
109− } else {
110− s.User = u
111− }
112− ts, _ := strconv.Atoi(chunks[2])
113− s.ExpiresOn = time.Unix(int64(ts), 0)
114− return
117+
118+ return sessionState, nil
115119}
providers/session_state_test.go+69−5
@@ -1,6 +1,7 @@
11package providers
22
33import (
4+ "fmt"
45 "strings"
56 "testing"
67 "time"
@@ -30,6 +31,7 @@ func TestSessionStateSerialization(t *testing.T) {
3031 ss, err := DecodeSessionState(encoded, c)
3132 t.Logf("%#v", ss)
3233 assert.Equal(t, nil, err)
34+ assert.Equal(t, "user", ss.User)
3335 assert.Equal(t, s.Email, ss.Email)
3436 assert.Equal(t, s.AccessToken, ss.AccessToken)
3537 assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix())
@@ -39,14 +41,50 @@ func TestSessionStateSerialization(t *testing.T) {
3941 ss, err = DecodeSessionState(encoded, c2)
4042 t.Logf("%#v", ss)
4143 assert.Equal(t, nil, err)
44+ assert.Equal(t, "user", ss.User)
4245 assert.Equal(t, s.Email, ss.Email)
4346 assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix())
4447 assert.NotEqual(t, s.AccessToken, ss.AccessToken)
4548 assert.NotEqual(t, s.RefreshToken, ss.RefreshToken)
4649}
4750
48−func TestSessionStateSerializationNoCipher(t *testing.T) {
51+func TestSessionStateSerializationWithUser(t *testing.T) {
52+ c, err := cookie.NewCipher([]byte(secret))
53+ assert.Equal(t, nil, err)
54+ c2, err := cookie.NewCipher([]byte(altSecret))
55+ assert.Equal(t, nil, err)
56+ s := &SessionState{
57+ User: "just-user",
58+ Email: "user@domain.com",
59+ AccessToken: "token1234",
60+ ExpiresOn: time.Now().Add(time.Duration(1) * time.Hour),
61+ RefreshToken: "refresh4321",
62+ }
63+ encoded, err := s.EncodeSessionState(c)
64+ assert.Equal(t, nil, err)
65+ assert.Equal(t, 3, strings.Count(encoded, "|"))
66+
67+ ss, err := DecodeSessionState(encoded, c)
68+ t.Logf("%#v", ss)
69+ assert.Equal(t, nil, err)
70+ assert.Equal(t, s.User, ss.User)
71+ assert.Equal(t, s.Email, ss.Email)
72+ assert.Equal(t, s.AccessToken, ss.AccessToken)
73+ assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix())
74+ assert.Equal(t, s.RefreshToken, ss.RefreshToken)
75+
76+ // ensure a different cipher can't decode properly (ie: it gets gibberish)
77+ ss, err = DecodeSessionState(encoded, c2)
78+ t.Logf("%#v", ss)
79+ assert.Equal(t, nil, err)
80+ assert.Equal(t, s.User, ss.User)
81+ assert.Equal(t, s.Email, ss.Email)
82+ assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix())
83+ assert.NotEqual(t, s.AccessToken, ss.AccessToken)
84+ assert.NotEqual(t, s.RefreshToken, ss.RefreshToken)
85+}
4986
87+func TestSessionStateSerializationNoCipher(t *testing.T) {
5088 s := &SessionState{
5189 Email: "user@domain.com",
5290 AccessToken: "token1234",
@@ -55,25 +93,51 @@ func TestSessionStateSerializationNoCipher(t *testing.T) {
5593 }
5694 encoded, err := s.EncodeSessionState(nil)
5795 assert.Equal(t, nil, err)
58− assert.Equal(t, s.Email, encoded)
96+ expected := fmt.Sprintf("email:%s user:", s.Email)
97+ assert.Equal(t, expected, encoded)
5998
6099 // only email should have been serialized
61100 ss, err := DecodeSessionState(encoded, nil)
62101 assert.Equal(t, nil, err)
102+ assert.Equal(t, "user", ss.User)
63103 assert.Equal(t, s.Email, ss.Email)
64104 assert.Equal(t, "", ss.AccessToken)
65105 assert.Equal(t, "", ss.RefreshToken)
66106}
67107
68−func TestSessionStateUserOrEmail(t *testing.T) {
108+func TestSessionStateSerializationNoCipherWithUser(t *testing.T) {
109+ s := &SessionState{
110+ User: "just-user",
111+ Email: "user@domain.com",
112+ AccessToken: "token1234",
113+ ExpiresOn: time.Now().Add(time.Duration(1) * time.Hour),
114+ RefreshToken: "refresh4321",
115+ }
116+ encoded, err := s.EncodeSessionState(nil)
117+ assert.Equal(t, nil, err)
118+ expected := fmt.Sprintf("email:%s user:%s", s.Email, s.User)
119+ assert.Equal(t, expected, encoded)
120+
121+ // only email should have been serialized
122+ ss, err := DecodeSessionState(encoded, nil)
123+ assert.Equal(t, nil, err)
124+ assert.Equal(t, s.User, ss.User)
125+ assert.Equal(t, s.Email, ss.Email)
126+ assert.Equal(t, "", ss.AccessToken)
127+ assert.Equal(t, "", ss.RefreshToken)
128+}
69129
130+func TestSessionStateAccountInfo(t *testing.T) {
70131 s := &SessionState{
71132 Email: "user@domain.com",
72133 User: "just-user",
73134 }
74− assert.Equal(t, "user@domain.com", s.userOrEmail())
135+ expected := fmt.Sprintf("email:%v user:%v", s.Email, s.User)
136+ assert.Equal(t, expected, s.accountInfo())
137+
75138 s.Email = ""
76− assert.Equal(t, "just-user", s.userOrEmail())
139+ expected = fmt.Sprintf("email:%v user:%v", s.Email, s.User)
140+ assert.Equal(t, expected, s.accountInfo())
77141}
78142
79143func TestExpired(t *testing.T) {