-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
connector: add uaa connector #542
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,158 @@ | ||
package connector | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"html/template" | ||
"net/http" | ||
"net/url" | ||
"path" | ||
|
||
chttp "github.com/coreos/go-oidc/http" | ||
"github.com/coreos/go-oidc/oauth2" | ||
"github.com/coreos/go-oidc/oidc" | ||
) | ||
|
||
const ( | ||
UAAConnectorType = "uaa" | ||
) | ||
|
||
type UAAConnectorConfig struct { | ||
ID string `json:"id"` | ||
ClientID string `json:"clientID"` | ||
ClientSecret string `json:"clientSecret"` | ||
ServerURL string `json:"serverURL"` | ||
} | ||
|
||
// standard error form returned by UAA | ||
type uaaError struct { | ||
ErrorDescription string `json:"error_description"` | ||
ErrorType string `json:"error"` | ||
} | ||
|
||
type uaaOAuth2Connector struct { | ||
clientID string | ||
clientSecret string | ||
client *oauth2.Client | ||
uaaBaseURL *url.URL | ||
} | ||
|
||
func init() { | ||
RegisterConnectorConfigType(UAAConnectorType, func() ConnectorConfig { return &UAAConnectorConfig{} }) | ||
} | ||
|
||
func (cfg *UAAConnectorConfig) ConnectorID() string { | ||
return cfg.ID | ||
} | ||
|
||
func (cfg *UAAConnectorConfig) ConnectorType() string { | ||
return UAAConnectorType | ||
} | ||
|
||
func (cfg *UAAConnectorConfig) Connector(ns url.URL, lf oidc.LoginFunc, tpls *template.Template) (Connector, error) { | ||
uaaBaseURL, err := url.ParseRequestURI(cfg.ServerURL) | ||
if err != nil { | ||
return nil, fmt.Errorf("Invalid configuration. UAA URL is invalid: %v", err) | ||
} | ||
if !uaaBaseURL.IsAbs() { | ||
return nil, fmt.Errorf("Invalid configuration. UAA URL must be absolute") | ||
} | ||
ns.Path = path.Join(ns.Path, httpPathCallback) | ||
oauth2Conn, err := newUAAConnector(cfg, uaaBaseURL, ns.String()) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return &OAuth2Connector{ | ||
id: cfg.ID, | ||
loginFunc: lf, | ||
cbURL: ns, | ||
conn: oauth2Conn, | ||
}, nil | ||
} | ||
|
||
func (err uaaError) Error() string { | ||
return fmt.Sprintf("uaa (%s): %s", err.ErrorType, err.ErrorDescription) | ||
} | ||
|
||
func (c *uaaOAuth2Connector) Client() *oauth2.Client { | ||
return c.client | ||
} | ||
|
||
func (c *uaaOAuth2Connector) Healthy() error { | ||
return nil | ||
} | ||
|
||
func (c *uaaOAuth2Connector) Identity(cli chttp.Client) (oidc.Identity, error) { | ||
uaaUserInfoURL := *c.uaaBaseURL | ||
uaaUserInfoURL.Path = path.Join(uaaUserInfoURL.Path, "/userinfo") | ||
req, err := http.NewRequest("GET", uaaUserInfoURL.String(), nil) | ||
if err != nil { | ||
return oidc.Identity{}, err | ||
} | ||
resp, err := cli.Do(req) | ||
if err != nil { | ||
return oidc.Identity{}, fmt.Errorf("get: %v", err) | ||
} | ||
defer resp.Body.Close() | ||
switch { | ||
case resp.StatusCode >= 400 && resp.StatusCode < 600: | ||
// attempt to decode error from UAA | ||
var authErr uaaError | ||
if err := json.NewDecoder(resp.Body).Decode(&authErr); err != nil { | ||
return oidc.Identity{}, oauth2.NewError(oauth2.ErrorAccessDenied) | ||
} | ||
return oidc.Identity{}, authErr | ||
case resp.StatusCode == http.StatusOK: | ||
default: | ||
return oidc.Identity{}, fmt.Errorf("unexpected status from providor %s", resp.Status) | ||
} | ||
var user struct { | ||
UserID string `json:"user_id"` | ||
Email string `json:"email"` | ||
Name string `json:"name"` | ||
UserName string `json:"user_name"` | ||
} | ||
if err := json.NewDecoder(resp.Body).Decode(&user); err != nil { | ||
return oidc.Identity{}, fmt.Errorf("getting user info: %v", err) | ||
} | ||
name := user.Name | ||
if name == "" { | ||
name = user.UserName | ||
} | ||
return oidc.Identity{ | ||
ID: user.UserID, | ||
Name: name, | ||
Email: user.Email, | ||
}, nil | ||
} | ||
|
||
func (c *uaaOAuth2Connector) TrustedEmailProvider() bool { | ||
return false | ||
} | ||
|
||
func newUAAConnector(cfg *UAAConnectorConfig, uaaBaseURL *url.URL, cbURL string) (oauth2Connector, error) { | ||
uaaAuthURL := *uaaBaseURL | ||
uaaTokenURL := *uaaBaseURL | ||
uaaAuthURL.Path = path.Join(uaaAuthURL.Path, "/oauth/authorize") | ||
uaaTokenURL.Path = path.Join(uaaTokenURL.Path, "/oauth/token") | ||
config := oauth2.Config{ | ||
Credentials: oauth2.ClientCredentials{ID: cfg.ClientID, Secret: cfg.ClientSecret}, | ||
AuthURL: uaaAuthURL.String(), | ||
TokenURL: uaaTokenURL.String(), | ||
Scope: []string{"openid"}, | ||
AuthMethod: oauth2.AuthMethodClientSecretPost, | ||
RedirectURL: cbURL, | ||
} | ||
|
||
cli, err := oauth2.NewClient(http.DefaultClient, config) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return &uaaOAuth2Connector{ | ||
clientID: cfg.ClientID, | ||
clientSecret: cfg.ClientSecret, | ||
client: cli, | ||
uaaBaseURL: uaaBaseURL, | ||
}, nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
package connector | ||
|
||
import ( | ||
"testing" | ||
) | ||
|
||
func TestUAAConnectorConfigInvalidserverURLNotAValidURL(t *testing.T) { | ||
cc := UAAConnectorConfig{ | ||
ID: "uaa", | ||
ClientID: "test-client", | ||
ClientSecret: "test-client-secret", | ||
ServerURL: "https//login.apigee.com", | ||
} | ||
|
||
_, err := cc.Connector(ns, lf, templates) | ||
if err == nil { | ||
t.Fatal("Expected UAAConnector initialization to fail when UAA URL is an invalid URL") | ||
} | ||
} | ||
|
||
func TestUAAConnectorConfigInvalidserverURLNotAbsolute(t *testing.T) { | ||
cc := UAAConnectorConfig{ | ||
ID: "uaa", | ||
ClientID: "test-client", | ||
ClientSecret: "test-client-secret", | ||
ServerURL: "/uaa", | ||
} | ||
|
||
_, err := cc.Connector(ns, lf, templates) | ||
if err == nil { | ||
t.Fatal("Expected UAAConnector initialization to fail when UAA URL is not an aboslute URL") | ||
} | ||
} | ||
|
||
func TestUAAConnectorConfigValidserverURL(t *testing.T) { | ||
cc := UAAConnectorConfig{ | ||
ID: "uaa", | ||
ClientID: "test-client", | ||
ClientSecret: "test-client-secret", | ||
ServerURL: "https://login.apigee.com", | ||
} | ||
|
||
_, err := cc.Connector(ns, lf, templates) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason this is always the empty string. Any idea why this is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's just the way the test data for UAA is created. When I test against a live server,
name
is populated as expected. You can hit the/uaa/userinfo
API directly to see ifname
is set or not but I don't think it is based on the UAA docs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wrong. Below is the response from
/uaa/userinfo
formarissa
:I'll see what is up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can verify that the
Name
is being parsed properly and given tooidc.Identity
. Still digging.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the example-app UI is suggesting that
connector/connector_uaa.go#L118
is""
but it's not. When we create theoidc.Identity
, the name is properly set toMarissa Bloggs
. It seems that when the example-application creates the claims object which gets rendered, it isn't finding this. I'll keep digging but the UAA Connector itself is working properly at the line you referenced.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing to note is that the example-app isn't printing out the details of the
oidc.Identity
created by the connector but is instead printing out the details of the OAuth2 token provided by the provider.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked the team working on UAA and they said
name
is not a standard field: https://self-issued.info/docs/draft-ietf-oauth-json-web-token.html#rfc.section.4.1There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an openid connect thing https://openid.net/specs/openid-connect-basic-1_0.html#StandardClaims
Not a blocker, just wondering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right but that is for the
/uaa/userinfo
response and thename
is set there. Theoidc.Identity
object that we craft as a result of calling the UserInfo endpoint does haveName
set properly. If you debug the line you pointed to, you'll see this is the case.What I'm saying is that the JWT token you're inspecting in the example-app is an OAuth2 token and
name
is not a standard field for that response.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was fixed by #537