Commit e6e5cc6e by Arve Knudsen Committed by GitHub

Chore: Simplify generic OAuth code (#26779)

* Generic OAuth: Simplify

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* Rename test variables for consistency

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
parent b646a1d6
......@@ -36,6 +36,13 @@ func New(logger string, ctx ...interface{}) Logger {
return Root.New(params...)
}
// NewWithLevel returns a new logger with a certain level.
func NewWithLevel(name string, level log15.Lvl) Logger {
logger := Root.New("logger", name)
logger.SetHandler(log15.LvlFilterHandler(level, log15.StreamHandler(os.Stdout, getLogFormat("console"))))
return logger
}
func Tracef(format string, v ...interface{}) {
var message string
if len(v) > 0 {
......
......@@ -82,6 +82,7 @@ type UserInfoJson struct {
Upn string `json:"upn"`
Attributes map[string][]string `json:"attributes"`
rawJSON []byte
source string
}
func (info *UserInfoJson) String() string {
......@@ -91,64 +92,97 @@ func (info *UserInfoJson) String() string {
}
func (s *SocialGenericOAuth) UserInfo(client *http.Client, token *oauth2.Token) (*BasicUserInfo, error) {
var data UserInfoJson
var err error
s.log.Debug("Getting user info")
tokenData := s.extractFromToken(token)
apiData := s.extractFromAPI(client)
userInfo := &BasicUserInfo{}
if s.extractToken(&data, token) {
s.fillUserInfo(userInfo, &data)
for _, data := range []*UserInfoJson{tokenData, apiData} {
if data == nil {
continue
}
if s.extractAPI(&data, client) {
s.fillUserInfo(userInfo, &data)
}
s.log.Debug("Processing external user info", "source", data.source, "data", data)
if userInfo.Email == "" {
userInfo.Email, err = s.FetchPrivateEmail(client)
if err != nil {
return nil, err
if userInfo.Name == "" {
if data.Name != "" {
s.log.Debug("Setting user info name from name field")
userInfo.Name = data.Name
} else if data.DisplayName != "" {
s.log.Debug("Setting user info name from display name field")
userInfo.Name = data.DisplayName
}
}
if userInfo.Login == "" {
userInfo.Login = userInfo.Email
if data.Login != "" {
s.log.Debug("Setting user info login from login field", "login", data.Login)
userInfo.Login = data.Login
} else {
if s.loginAttributePath != "" {
s.log.Debug("Searching for login among JSON", "loginAttributePath", s.loginAttributePath)
login, err := s.searchJSONForAttr(s.loginAttributePath, data.rawJSON)
if err != nil {
s.log.Error("Failed to search JSON for login attribute", "error", err)
} else if login != "" {
userInfo.Login = login
s.log.Debug("Setting user info login from login field", "login", login)
}
if !s.IsTeamMember(client) {
return nil, errors.New("User not a member of one of the required teams")
}
if !s.IsOrganizationMember(client) {
return nil, errors.New("User not a member of one of the required organizations")
if userInfo.Login == "" && data.Username != "" {
s.log.Debug("Setting user info login from username field", "username", data.Username)
userInfo.Login = data.Username
}
}
}
s.log.Debug("User info result", "result", userInfo)
return userInfo, nil
}
func (s *SocialGenericOAuth) fillUserInfo(userInfo *BasicUserInfo, data *UserInfoJson) {
if userInfo.Email == "" {
userInfo.Email = s.extractEmail(data)
if userInfo.Email != "" {
s.log.Debug("Set user info email from extracted email", "email", userInfo.Email)
}
}
if userInfo.Role == "" {
role, err := s.extractRole(data)
if err != nil {
s.log.Error("Failed to extract role", "error", err)
} else {
} else if role != "" {
s.log.Debug("Setting user info role from extracted role")
userInfo.Role = role
}
}
if userInfo.Name == "" {
userInfo.Name = s.extractName(data)
}
if userInfo.Email == "" {
var err error
userInfo.Email, err = s.FetchPrivateEmail(client)
if err != nil {
return nil, err
}
s.log.Debug("Setting email from fetched private email", "email", userInfo.Email)
}
if userInfo.Login == "" {
userInfo.Login = s.extractLogin(data)
s.log.Debug("Defaulting to using email for user info login", "email", userInfo.Email)
userInfo.Login = userInfo.Email
}
if !s.IsTeamMember(client) {
return nil, errors.New("user not a member of one of the required teams")
}
if !s.IsOrganizationMember(client) {
return nil, errors.New("user not a member of one of the required organizations")
}
s.log.Debug("User info result", "result", userInfo)
return userInfo, nil
}
func (s *SocialGenericOAuth) extractToken(data *UserInfoJson, token *oauth2.Token) bool {
var err error
func (s *SocialGenericOAuth) extractFromToken(token *oauth2.Token) *UserInfoJson {
s.log.Debug("Extracting user info from OAuth token")
idTokenAttribute := "id_token"
if s.idTokenAttributeName != "" {
......@@ -159,50 +193,54 @@ func (s *SocialGenericOAuth) extractToken(data *UserInfoJson, token *oauth2.Toke
idToken := token.Extra(idTokenAttribute)
if idToken == nil {
s.log.Debug("No id_token found", "token", token)
return false
return nil
}
jwtRegexp := regexp.MustCompile("^([-_a-zA-Z0-9=]+)[.]([-_a-zA-Z0-9=]+)[.]([-_a-zA-Z0-9=]+)$")
matched := jwtRegexp.FindStringSubmatch(idToken.(string))
if matched == nil {
s.log.Debug("id_token is not in JWT format", "id_token", idToken.(string))
return false
return nil
}
data.rawJSON, err = base64.RawURLEncoding.DecodeString(matched[2])
rawJSON, err := base64.RawURLEncoding.DecodeString(matched[2])
if err != nil {
s.log.Error("Error base64 decoding id_token", "raw_payload", matched[2], "error", err)
return false
return nil
}
err = json.Unmarshal(data.rawJSON, data)
if err != nil {
var data UserInfoJson
if err := json.Unmarshal(rawJSON, &data); err != nil {
s.log.Error("Error decoding id_token JSON", "raw_json", string(data.rawJSON), "error", err)
data.rawJSON = []byte{}
return false
return nil
}
data.rawJSON = rawJSON
data.source = "token"
s.log.Debug("Received id_token", "raw_json", string(data.rawJSON), "data", data)
return true
return &data
}
func (s *SocialGenericOAuth) extractAPI(data *UserInfoJson, client *http.Client) bool {
func (s *SocialGenericOAuth) extractFromAPI(client *http.Client) *UserInfoJson {
s.log.Debug("Getting user info from API")
rawUserInfoResponse, err := HttpGet(client, s.apiUrl)
if err != nil {
s.log.Debug("Error getting user info", "url", s.apiUrl, "error", err)
return false
s.log.Debug("Error getting user info from API", "url", s.apiUrl, "error", err)
return nil
}
data.rawJSON = rawUserInfoResponse.Body
err = json.Unmarshal(data.rawJSON, data)
if err != nil {
s.log.Error("Error decoding user info response", "raw_json", data.rawJSON, "error", err)
data.rawJSON = []byte{}
return false
rawJSON := rawUserInfoResponse.Body
var data UserInfoJson
if err := json.Unmarshal(rawJSON, &data); err != nil {
s.log.Error("Error decoding user info response", "raw_json", rawJSON, "error", err)
return nil
}
s.log.Debug("Received user info response", "raw_json", string(data.rawJSON), "data", data)
return true
data.rawJSON = rawJSON
data.source = "API"
s.log.Debug("Received user info response from API", "raw_json", string(rawJSON), "data", data)
return &data
}
func (s *SocialGenericOAuth) extractEmail(data *UserInfoJson) string {
......@@ -247,39 +285,6 @@ func (s *SocialGenericOAuth) extractRole(data *UserInfoJson) (string, error) {
return role, nil
}
func (s *SocialGenericOAuth) extractLogin(data *UserInfoJson) string {
if data.Login != "" {
return data.Login
}
if s.loginAttributePath != "" {
login, err := s.searchJSONForAttr(s.loginAttributePath, data.rawJSON)
if err != nil {
s.log.Error("Failed to search JSON for attribute", "error", err)
} else if login != "" {
return login
}
}
if data.Username != "" {
return data.Username
}
return ""
}
func (s *SocialGenericOAuth) extractName(data *UserInfoJson) string {
if data.Name != "" {
return data.Name
}
if data.DisplayName != "" {
return data.DisplayName
}
return ""
}
func (s *SocialGenericOAuth) FetchPrivateEmail(client *http.Client) (string, error) {
type Record struct {
Email string `json:"email"`
......
......@@ -6,6 +6,7 @@ import (
"net/http/httptest"
"time"
"github.com/inconshreveable/log15"
"github.com/stretchr/testify/require"
"testing"
......@@ -18,7 +19,7 @@ func TestSearchJSONForEmail(t *testing.T) {
t.Run("Given a generic OAuth provider", func(t *testing.T) {
provider := SocialGenericOAuth{
SocialBase: &SocialBase{
log: log.New("generic_oauth_test"),
log: log.NewWithLevel("generic_oauth_test", log15.LvlDebug),
},
}
......@@ -106,7 +107,7 @@ func TestSearchJSONForRole(t *testing.T) {
t.Run("Given a generic OAuth provider", func(t *testing.T) {
provider := SocialGenericOAuth{
SocialBase: &SocialBase{
log: log.New("generic_oauth_test"),
log: log.NewWithLevel("generic_oauth_test", log15.LvlDebug),
},
}
......@@ -169,21 +170,21 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) {
t.Run("Given a generic OAuth provider", func(t *testing.T) {
provider := SocialGenericOAuth{
SocialBase: &SocialBase{
log: log.New("generic_oauth_test"),
log: log.NewWithLevel("generic_oauth_test", log15.LvlDebug),
},
emailAttributePath: "email",
}
tests := []struct {
Name string
APIURLResponse interface{}
ResponseBody interface{}
OAuth2Extra interface{}
RoleAttributePath string
ExpectedEmail string
ExpectedRole string
}{
{
Name: "Given a valid id_token, a valid role path, no api response, use id_token",
Name: "Given a valid id_token, a valid role path, no API response, use id_token",
OAuth2Extra: map[string]interface{}{
// { "role": "Admin", "email": "john.doe@example.com" }
"id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoiQWRtaW4iLCJlbWFpbCI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIn0.9PtHcCaXxZa2HDlASyKIaFGfOKlw2ILQo32xlvhvhRg",
......@@ -193,7 +194,7 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) {
ExpectedRole: "Admin",
},
{
Name: "Given a valid id_token, no role path, no api response, use id_token",
Name: "Given a valid id_token, no role path, no API response, use id_token",
OAuth2Extra: map[string]interface{}{
// { "email": "john.doe@example.com" }
"id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIn0.k5GwPcZvGe2BE_jgwN0ntz0nz4KlYhEd0hRRLApkTJ4",
......@@ -203,7 +204,7 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) {
ExpectedRole: "",
},
{
Name: "Given a valid id_token, an invalid role path, no api response, use id_token",
Name: "Given a valid id_token, an invalid role path, no API response, use id_token",
OAuth2Extra: map[string]interface{}{
// { "role": "Admin", "email": "john.doe@example.com" }
"id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoiQWRtaW4iLCJlbWFpbCI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIn0.9PtHcCaXxZa2HDlASyKIaFGfOKlw2ILQo32xlvhvhRg",
......@@ -213,8 +214,8 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) {
ExpectedRole: "",
},
{
Name: "Given no id_token, a valid role path, a valid api response, use api response",
APIURLResponse: map[string]interface{}{
Name: "Given no id_token, a valid role path, a valid API response, use API response",
ResponseBody: map[string]interface{}{
"role": "Admin",
"email": "john.doe@example.com",
},
......@@ -223,8 +224,8 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) {
ExpectedRole: "Admin",
},
{
Name: "Given no id_token, no role path, a valid api response, use api response",
APIURLResponse: map[string]interface{}{
Name: "Given no id_token, no role path, a valid API response, use API response",
ResponseBody: map[string]interface{}{
"email": "john.doe@example.com",
},
RoleAttributePath: "",
......@@ -232,8 +233,8 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) {
ExpectedRole: "",
},
{
Name: "Given no id_token, a role path, a valid api response without a role, use api response",
APIURLResponse: map[string]interface{}{
Name: "Given no id_token, a role path, a valid API response without a role, use API response",
ResponseBody: map[string]interface{}{
"email": "john.doe@example.com",
},
RoleAttributePath: "role",
......@@ -241,18 +242,18 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) {
ExpectedRole: "",
},
{
Name: "Given no id_token, a valid role path, no api response, no data",
Name: "Given no id_token, a valid role path, no API response, no data",
RoleAttributePath: "role",
ExpectedEmail: "",
ExpectedRole: "",
},
{
Name: "Given a valid id_token, a valid role path, a valid api response, prefer id_token",
Name: "Given a valid id_token, a valid role path, a valid API response, prefer id_token",
OAuth2Extra: map[string]interface{}{
// { "role": "Admin", "email": "john.doe@example.com" }
"id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoiQWRtaW4iLCJlbWFpbCI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIn0.9PtHcCaXxZa2HDlASyKIaFGfOKlw2ILQo32xlvhvhRg",
},
APIURLResponse: map[string]interface{}{
ResponseBody: map[string]interface{}{
"role": "FromResponse",
"email": "from_response@example.com",
},
......@@ -261,12 +262,12 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) {
ExpectedRole: "Admin",
},
{
Name: "Given a valid id_token, an invalid role path, a valid api response, prefer id_token",
Name: "Given a valid id_token, an invalid role path, a valid API response, prefer id_token",
OAuth2Extra: map[string]interface{}{
// { "role": "Admin", "email": "john.doe@example.com" }
"id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoiQWRtaW4iLCJlbWFpbCI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIn0.9PtHcCaXxZa2HDlASyKIaFGfOKlw2ILQo32xlvhvhRg",
},
APIURLResponse: map[string]interface{}{
ResponseBody: map[string]interface{}{
"role": "FromResponse",
"email": "from_response@example.com",
},
......@@ -275,12 +276,12 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) {
ExpectedRole: "",
},
{
Name: "Given a valid id_token with no email, a valid role path, a valid api response with no role, merge",
Name: "Given a valid id_token with no email, a valid role path, a valid API response with no role, merge",
OAuth2Extra: map[string]interface{}{
// { "role": "Admin" }
"id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoiQWRtaW4ifQ.k5GwPcZvGe2BE_jgwN0ntz0nz4KlYhEd0hRRLApkTJ4",
},
APIURLResponse: map[string]interface{}{
ResponseBody: map[string]interface{}{
"email": "from_response@example.com",
},
RoleAttributePath: "role",
......@@ -288,12 +289,12 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) {
ExpectedRole: "Admin",
},
{
Name: "Given a valid id_token with no role, a valid role path, a valid api response with no email, merge",
Name: "Given a valid id_token with no role, a valid role path, a valid API response with no email, merge",
OAuth2Extra: map[string]interface{}{
// { "email": "john.doe@example.com" }
"id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIn0.k5GwPcZvGe2BE_jgwN0ntz0nz4KlYhEd0hRRLApkTJ4",
},
APIURLResponse: map[string]interface{}{
ResponseBody: map[string]interface{}{
"role": "FromResponse",
},
RoleAttributePath: "role",
......@@ -305,12 +306,12 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) {
for _, test := range tests {
provider.roleAttributePath = test.RoleAttributePath
t.Run(test.Name, func(t *testing.T) {
response, err := json.Marshal(test.APIURLResponse)
body, err := json.Marshal(test.ResponseBody)
require.NoError(t, err)
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
w.Header().Set("Content-Type", "application/json")
_, err = w.Write(response)
_, err = w.Write(body)
require.NoError(t, err)
}))
provider.apiUrl = ts.URL
......@@ -336,20 +337,20 @@ func TestUserInfoSearchesForLogin(t *testing.T) {
t.Run("Given a generic OAuth provider", func(t *testing.T) {
provider := SocialGenericOAuth{
SocialBase: &SocialBase{
log: log.New("generic_oauth_test"),
log: log.NewWithLevel("generic_oauth_test", log15.LvlDebug),
},
loginAttributePath: "login",
}
tests := []struct {
Name string
APIURLResponse interface{}
ResponseBody interface{}
OAuth2Extra interface{}
LoginAttributePath string
ExpectedLogin string
}{
{
Name: "Given a valid id_token, a valid login path, no api response, use id_token",
Name: "Given a valid id_token, a valid login path, no API response, use id_token",
OAuth2Extra: map[string]interface{}{
// { "login": "johndoe", "email": "john.doe@example.com" }
"id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJsb2dpbiI6ImpvaG5kb2UiLCJlbWFpbCI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIn0.sg4sRJCNpax_76XMgr277fdxhjjtNSWXKIOFv4_GJN8",
......@@ -358,7 +359,7 @@ func TestUserInfoSearchesForLogin(t *testing.T) {
ExpectedLogin: "johndoe",
},
{
Name: "Given a valid id_token, no login path, no api response, use id_token",
Name: "Given a valid id_token, no login path, no API response, use id_token",
OAuth2Extra: map[string]interface{}{
// { "login": "johndoe", "email": "john.doe@example.com" }
"id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJsb2dpbiI6ImpvaG5kb2UiLCJlbWFpbCI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIn0.sg4sRJCNpax_76XMgr277fdxhjjtNSWXKIOFv4_GJN8",
......@@ -367,8 +368,8 @@ func TestUserInfoSearchesForLogin(t *testing.T) {
ExpectedLogin: "johndoe",
},
{
Name: "Given no id_token, a valid login path, a valid api response, use api response",
APIURLResponse: map[string]interface{}{
Name: "Given no id_token, a valid login path, a valid API response, use API response",
ResponseBody: map[string]interface{}{
"user_uid": "johndoe",
"email": "john.doe@example.com",
},
......@@ -376,23 +377,23 @@ func TestUserInfoSearchesForLogin(t *testing.T) {
ExpectedLogin: "johndoe",
},
{
Name: "Given no id_token, no login path, a valid api response, use api response",
APIURLResponse: map[string]interface{}{
Name: "Given no id_token, no login path, a valid API response, use API response",
ResponseBody: map[string]interface{}{
"login": "johndoe",
},
LoginAttributePath: "",
ExpectedLogin: "johndoe",
},
{
Name: "Given no id_token, a login path, a valid api response without a login, use api response",
APIURLResponse: map[string]interface{}{
Name: "Given no id_token, a login path, a valid API response without a login, use API response",
ResponseBody: map[string]interface{}{
"username": "john.doe",
},
LoginAttributePath: "login",
ExpectedLogin: "john.doe",
},
{
Name: "Given no id_token, a valid login path, no api response, no data",
Name: "Given no id_token, a valid login path, no API response, no data",
LoginAttributePath: "login",
ExpectedLogin: "",
},
......@@ -401,12 +402,13 @@ func TestUserInfoSearchesForLogin(t *testing.T) {
for _, test := range tests {
provider.loginAttributePath = test.LoginAttributePath
t.Run(test.Name, func(t *testing.T) {
response, err := json.Marshal(test.APIURLResponse)
body, err := json.Marshal(test.ResponseBody)
require.NoError(t, err)
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
w.Header().Set("Content-Type", "application/json")
_, err = w.Write(response)
t.Log("Writing fake API response body", "body", test.ResponseBody)
_, err = w.Write(body)
require.NoError(t, err)
}))
provider.apiUrl = ts.URL
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment