add state variable, bug fixes [#523]

.secrets.baseline

@@ -133,7 +133,7 @@
         "filename": "core/auth/auth.go",
         "hashed_secret": "4d55af37dbbb6a42088d917caa1ca25428ec42c9",
         "is_verified": false,
-        "line_number": 2393
+        "line_number": 2420
       }
     ],
     "core/auth/auth_type_email.go": [
@@ -193,7 +193,7 @@
         "filename": "core/auth/auth_type_oauth2.go",
         "hashed_secret": "15a46c63d80cdc62bb7e988a24b5839ecb624e25",
         "is_verified": false,
-        "line_number": 236
+        "line_number": 242
       }
     ],
     "core/auth/auth_type_oidc.go": [
@@ -288,5 +288,5 @@
       }
     ]
   },
-  "generated_at": "2022-08-22T22:23:14Z"
+  "generated_at": "2022-08-23T17:19:19Z"
 }

core/auth/apis.go

@@ -562,7 +562,7 @@ func (a *Auth) LoginMFA(apiKey string, accountID string, sessionID string, ident
 func (a *Auth) CreateAdminAccount(authenticationType string, appID string, orgID string, identifier string, profile model.Profile,
 	permissions []string, roleIDs []string, groupIDs []string, creatorPermissions []string, l *logs.Log) (*model.Account, map[string]interface{}, error) {
 	//TODO: add admin authentication policies that specify which auth types may be used for each app org
-	if authenticationType != AuthTypeOidc && authenticationType != AuthTypeEmail && !strings.HasSuffix(authenticationType, "_oidc") {
+	if !a.isValidAdminAuthType(authenticationType) {
 		return nil, nil, errors.ErrorData(logutils.StatusInvalid, "auth type", nil)
 	}
 
@@ -625,7 +625,7 @@ func (a *Auth) UpdateAdminAccount(authenticationType string, appID string, orgID
 	//TODO: when elevating existing accounts to application level admin, need to enforce any authentication policies set up for the app org
 	// when demoting from application level admin to standard user, may want to inform user of applicable authentication policy changes
 
-	if authenticationType != AuthTypeOidc && authenticationType != AuthTypeEmail && !strings.HasSuffix(authenticationType, "_oidc") {
+	if !a.isValidAdminAuthType(authenticationType) {
 		return nil, nil, errors.ErrorData(logutils.StatusInvalid, "auth type", nil)
 	}
 

core/auth/auth.go

@@ -22,6 +22,8 @@ import (
 	"crypto/rsa"
 	"encoding/json"
 	"fmt"
+	"net/url"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -2059,6 +2061,31 @@ func (a *Auth) validateAuthTypeForAppOrg(authenticationType string, appID string
 	return nil, nil, errors.ErrorData(logutils.StatusInvalid, typeAuthType, &logutils.FieldArgs{"app_org_id": appOrg.ID, "auth_type": authenticationType})
 }
 
+func (a *Auth) isValidAdminAuthType(authenticationType string) bool {
+	return authenticationType == AuthTypeOidc || authenticationType == AuthTypeOAuth2 || authenticationType == AuthTypeEmail || strings.HasSuffix(authenticationType, "_oidc") || strings.HasSuffix(authenticationType, "_oauth2")
+}
+
+func (a *Auth) queryValuesFromURL(urlStr string) (url.Values, error) {
+	unquotedCreds, err := strconv.Unquote(urlStr)
+	if err != nil {
+		return nil, errors.WrapErrorAction(logutils.ActionParse, "raw url", nil, err)
+	}
+	parsedURL, err := url.Parse(unquotedCreds)
+	if err != nil {
+		return nil, errors.WrapErrorAction(logutils.ActionParse, "unquoted url", nil, err)
+	}
+	unescapedQuery, err := url.QueryUnescape(parsedURL.RawQuery)
+	if err != nil {
+		return nil, errors.WrapErrorAction(logutils.ActionParse, "raw url query", nil, err)
+	}
+	parsedCreds, err := url.ParseQuery(unescapedQuery)
+	if err != nil {
+		return nil, errors.WrapErrorAction(logutils.ActionParse, "unescaped url query", nil, err)
+	}
+
+	return parsedCreds, nil
+}
+
 func (a *Auth) getAuthTypeImpl(authType model.AuthType) (authType, error) {
 	if auth, ok := a.authTypes[authType.Code]; ok {
 		return auth, nil

core/auth/auth_type_oauth2.go

@@ -48,16 +48,19 @@ type oauth2AuthImpl struct {
 }
 
 type oauth2AuthConfig struct {
-	Host         string            `json:"host" validate:"required"`
-	AuthorizeURL string            `json:"authorize_url"`
-	TokenURL     string            `json:"token_url"`
-	UserInfoURL  string            `json:"userinfo_url"`
-	Scopes       string            `json:"scopes"`
-	AllowSignUp  bool              `json:"allow_signup"`
-	UseState     bool              `json:"use_state"`
-	ClientID     string            `json:"client_id" validate:"required"`
-	ClientSecret string            `json:"client_secret" validate:"required"`
-	TokenTypes   map[string]string `json:"token_types" validaate:"required"`
+	Host         string `json:"host" validate:"required"`
+	AuthorizeURL string `json:"authorize_url"`
+	TokenURL     string `json:"token_url"`
+	UserInfoURL  string `json:"userinfo_url"`
+	Scopes       string `json:"scopes"`
+	AllowSignUp  bool   `json:"allow_signup"`
+	UseState     bool   `json:"use_state"`
+	ClientID     string `json:"client_id" validate:"required"`
+	ClientSecret string `json:"client_secret" validate:"required"`
+}
+
+type oauth2LoginParams struct {
+	State string `json:"state"`
 }
 
 type oauth2Token struct {
@@ -76,12 +79,30 @@ func (a *oauth2AuthImpl) externalLogin(authType model.AuthType, appType model.Ap
 		return nil, nil, errors.WrapErrorAction(logutils.ActionGet, typeOAuth2AuthConfig, nil, err)
 	}
 
-	parsedCreds, err := url.Parse(strings.ReplaceAll(creds, `"`, ""))
+	var loginParams oauth2LoginParams
+	if oauth2Config.UseState {
+		err := json.Unmarshal([]byte(params), &loginParams)
+		if err != nil {
+			return nil, nil, errors.WrapErrorAction(logutils.ActionUnmarshal, typeOAuth2LoginParams, nil, err)
+		}
+		validate := validator.New()
+		err = validate.Struct(loginParams)
+		if err != nil {
+			return nil, nil, errors.WrapErrorAction(logutils.ActionValidate, typeOAuth2LoginParams, nil, err)
+		}
+	}
+
+	parsedCreds, err := a.auth.queryValuesFromURL(creds)
 	if err != nil {
-		return nil, nil, errors.WrapErrorAction(logutils.ActionParse, "oauth2 login creds", nil, err)
+		return nil, nil, errors.WrapErrorAction(logutils.ActionParse, "oauth2 creds", nil, err)
+	}
+
+	//state in creds must match state generated for login url (if used)
+	if oauth2Config.UseState && loginParams.State != parsedCreds.Get("state") {
+		return nil, nil, errors.ErrorData(logutils.StatusInvalid, "oauth2 login", &logutils.FieldArgs{"state": parsedCreds.Get("state")})
 	}
 
-	externalUser, parameters, err := a.newToken(parsedCreds.Query().Get("code"), authType, appType, appOrg, oauth2Config, l)
+	externalUser, parameters, err := a.newToken(parsedCreds.Get("code"), authType, appType, appOrg, oauth2Config, l)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -99,7 +120,6 @@ func (a *oauth2AuthImpl) getLoginURL(authType model.AuthType, appType model.Appl
 		return "", nil, errors.WrapErrorAction(logutils.ActionGet, typeOAuth2AuthConfig, nil, err)
 	}
 
-	//TODO: necessary?
 	responseParams := map[string]interface{}{
 		"redirect_uri": redirectURI,
 	}
@@ -108,17 +128,16 @@ func (a *oauth2AuthImpl) getLoginURL(authType model.AuthType, appType model.Appl
 		"client_id":    oauth2Config.ClientID,
 		"redirect_uri": redirectURI,
 		"scope":        oauth2Config.Scopes,
-		// "login":        "", //TODO: prompt user with specific account to use (optional)
 		"allow_signup": strconv.FormatBool(oauth2Config.AllowSignUp),
 	}
 
-	//TODO: will need to store state variable to make use of this
 	if oauth2Config.UseState {
 		state, err := generateState()
 		if err != nil {
 			return "", nil, errors.WrapErrorAction("generating", "random state", nil, err)
 		}
 		bodyData["state"] = state
+		responseParams["state"] = state
 	}
 
 	authURL := oauth2Config.Host + "/login/oauth/authorize"
@@ -149,18 +168,10 @@ func (a *oauth2AuthImpl) loadOAuth2TokensAndInfo(bodyData map[string]string, oau
 		return nil, nil, errors.WrapErrorAction(logutils.ActionGet, typeOAuth2Token, nil, err)
 	}
 
-	// sub, err := a.checkToken(token.IDToken, authType, appType, oauth2Config, l)
-	// if err != nil {
-	// 	return nil, nil, errors.WrapErrorAction(logutils.ActionValidate, typeOAuth2Token, nil, err)
-	// }
-
 	userInfoURL := oauth2Config.Host + "/login/oauth/user"
 	if len(oauth2Config.UserInfoURL) > 0 {
 		userInfoURL = oauth2Config.UserInfoURL
 	}
-	if oauth2Config.TokenTypes[token.TokenType] != "" {
-		token.TokenType = oauth2Config.TokenTypes[token.TokenType]
-	}
 	userInfo, err := a.loadOAuth2UserInfo(token, userInfoURL)
 	if err != nil {
 		return nil, nil, errors.WrapErrorAction(logutils.ActionGet, "user info", nil, err)
@@ -172,11 +183,6 @@ func (a *oauth2AuthImpl) loadOAuth2TokensAndInfo(bodyData map[string]string, oau
 		return nil, nil, errors.WrapErrorAction(logutils.ActionUnmarshal, "user info", nil, err)
 	}
 
-	// userClaimsSub, _ := userClaims["sub"].(string)
-	// if userClaimsSub != sub {
-	// 	return nil, nil, errors.Newf("mismatching user info sub %s and id token sub %s", userClaimsSub, sub)
-	// }
-
 	identityProviderID, _ := authType.Params["identity_provider"].(string)
 	identityProviderSetting := appOrg.FindIdentityProviderSetting(identityProviderID)
 	if identityProviderSetting == nil {
@@ -187,7 +193,7 @@ func (a *oauth2AuthImpl) loadOAuth2TokensAndInfo(bodyData map[string]string, oau
 	identifier, _ := userClaims[identityProviderSetting.UserIdentifierField].(string)
 	//name
 	name, _ := userClaims[identityProviderSetting.NameField].(string)
-	names := strings.Split(name, "")
+	names := strings.Split(name, " ")
 	//email
 	email, _ := userClaims[identityProviderSetting.EmailField].(string)
 	//system specific
@@ -211,7 +217,7 @@ func (a *oauth2AuthImpl) loadOAuth2TokensAndInfo(bodyData map[string]string, oau
 	}
 
 	externalUser := model.ExternalSystemUser{Identifier: identifier, ExternalIDs: externalIDs, FirstName: names[0],
-		LastName: names[1], Email: email, SystemSpecific: systemSpecific}
+		LastName: names[len(names)-1], Email: email, SystemSpecific: systemSpecific}
 
 	oauth2Params := map[string]interface{}{}
 	oauth2Params["access_token"] = token.AccessToken

core/auth/auth_type_oidc.go

@@ -122,12 +122,12 @@ func (a *oidcAuthImpl) externalLogin(authType model.AuthType, appType model.Appl
 		return nil, nil, errors.WrapErrorAction(logutils.ActionGet, typeOidcAuthConfig, nil, err)
 	}
 
-	parsedCreds, err := url.Parse(strings.ReplaceAll(creds, `"`, ""))
+	parsedCreds, err := a.auth.queryValuesFromURL(creds)
 	if err != nil {
-		return nil, nil, errors.WrapErrorAction(logutils.ActionParse, "oidc login creds", nil, err)
+		return nil, nil, errors.WrapErrorAction(logutils.ActionParse, "oidc creds", nil, err)
 	}
 
-	externalUser, parameters, err := a.newToken(parsedCreds.Query().Get("code"), authType, appType, appOrg, &loginParams, oidcConfig, l)
+	externalUser, parameters, err := a.newToken(parsedCreds.Get("code"), authType, appType, appOrg, &loginParams, oidcConfig, l)
 	if err != nil {
 		return nil, nil, err
 	}

driver/web/docs/gen/def.yaml

@@ -5325,16 +5325,14 @@ components:
             - $ref: '#/components/schemas/_shared_req_CredsEmail'
             - $ref: '#/components/schemas/_shared_req_CredsTwilioPhone'
             - $ref: '#/components/schemas/_shared_req_CredsOIDC'
-            - type: string
-              description: |
-                Auth login creds for auth_type="oauth2" (or variants)
-                  - full redirect URI received from OAuth2 provider
+            - $ref: '#/components/schemas/_shared_req_CredsOAuth2'
             - $ref: '#/components/schemas/_shared_req_CredsAPIKey'
         params:
           type: object
           anyOf:
             - $ref: '#/components/schemas/_shared_req_ParamsEmail'
             - $ref: '#/components/schemas/_shared_req_ParamsOIDC'
+            - $ref: '#/components/schemas/_shared_req_ParamsOAuth2'
             - $ref: '#/components/schemas/_shared_req_ParamsNone'
         device:
           $ref: '#/components/schemas/_shared_req_Login_Device'
@@ -5449,6 +5447,7 @@ components:
           enum:
             - email
             - illinois_oidc
+            - github_oauth2
         identifier:
           type: string
         permissions:
@@ -5476,6 +5475,7 @@ components:
           enum:
             - email
             - illinois_oidc
+            - github_oauth2
         identifier:
           type: string
         permissions:
@@ -5506,6 +5506,7 @@ components:
             - email
             - twilio_phone
             - illinois_oidc
+            - github_oauth2
             - anonymous
         app_type_identifier:
           type: string
@@ -5541,6 +5542,11 @@ components:
       description: |
         Auth login creds for auth_type="oidc" (or variants)
           - full redirect URI received from OIDC provider
+    _shared_req_CredsOAuth2:
+      type: string
+      description: |
+        Auth login creds for auth_type="oauth2" (or variants)
+          - full redirect URI received from OAuth2 provider
     _shared_req_CredsAPIKey:
       type: object
       description: Auth login creds for auth_type="anonymous"
@@ -5565,6 +5571,12 @@ components:
           type: string
         pkce_verifier:
           type: string
+    _shared_req_ParamsOAuth2:
+      type: object
+      description: Auth login params for auth_type="oauth2" (or variants)
+      properties:
+        state:
+          type: string
     _shared_req_ParamsNone:
       type: object
       description: Auth login request params for unlisted auth_types (None)
@@ -5843,6 +5855,7 @@ components:
             - email
             - twilio_phone
             - illinois_oidc
+            - github_oauth2
             - username
         app_type_identifier:
           type: string
@@ -5870,6 +5883,7 @@ components:
             - email
             - twilio_phone
             - illinois_oidc
+            - github_oauth2
             - username
         app_type_identifier:
           type: string

driver/web/docs/schemas/apis/services/account/auth-type/link/request/Link.yaml

@@ -10,6 +10,7 @@ properties:
       - email
       - twilio_phone
       - illinois_oidc
+      - github_oauth2
       - username
   app_type_identifier:
     type: string