diff --git a/client/web/src/tracking/eventLogger.ts b/client/web/src/tracking/eventLogger.ts index 4385f4d9517..4b1d29fbfe9 100644 --- a/client/web/src/tracking/eventLogger.ts +++ b/client/web/src/tracking/eventLogger.ts @@ -477,11 +477,13 @@ export function setDebugEventLoggingEnabled(enabled: boolean): void { function handleQueryEvents(url: string): void { const parsedUrl = new URL(url) if (parsedUrl.searchParams.has('signup')) { - eventLogger.log('web:auth:signUpCompleted') + const args = { serviceType: parsedUrl.searchParams.get('signup') || '' } + eventLogger.log('web:auth:signUpCompleted', args, args) } if (parsedUrl.searchParams.has('signin')) { - eventLogger.log('web:auth:signInCompleted') + const args = { serviceType: parsedUrl.searchParams.get('signin') || '' } + eventLogger.log('web:auth:signInCompleted', args, args) } stripURLParameters(url, ['utm_campaign', 'utm_source', 'utm_medium', 'signup', 'signin']) diff --git a/cmd/frontend/auth/redirect.go b/cmd/frontend/auth/redirect.go index 15e0b64bc50..41f3d80f813 100644 --- a/cmd/frontend/auth/redirect.go +++ b/cmd/frontend/auth/redirect.go @@ -25,21 +25,21 @@ func SafeRedirectURL(urlStr string) string { } // Add a ?signup= or ?signin= parameter to a redirect URL. -func AddPostAuthRedirectParametersToURL(u *url.URL, newUserCreated bool) { +func AddPostAuthRedirectParametersToURL(u *url.URL, newUserCreated bool, serviceType string) { q := u.Query() if newUserCreated { - q.Add("signup", "") + q.Add("signup", serviceType) } else { - q.Add("signin", "") + q.Add("signin", serviceType) } u.RawQuery = q.Encode() } -func AddPostAuthRedirectParametersToString(urlStr string, newUserCreated bool) string { +func AddPostAuthRedirectParametersToString(urlStr string, newUserCreated bool, serviceType string) string { u, err := url.Parse(urlStr) if err != nil { return urlStr } - AddPostAuthRedirectParametersToURL(u, newUserCreated) + AddPostAuthRedirectParametersToURL(u, newUserCreated, serviceType) return u.String() } diff --git a/cmd/frontend/auth/user.go b/cmd/frontend/auth/user.go index 80c3e69297f..66c37e8dd3a 100644 --- a/cmd/frontend/auth/user.go +++ b/cmd/frontend/auth/user.go @@ -164,6 +164,9 @@ func GetAndSaveUser(ctx context.Context, db database.DB, op GetAndSaveUserOp) (n } const eventName = "ExternalAuthSignupSucceeded" + + // SECURITY: This args map is treated as a public argument in the LogEvent call below, so it must not contain + // any sensitive data. args, err := json.Marshal(map[string]any{ // NOTE: The conventional name should be "service_type", but keeping as-is for // backwards capability. @@ -185,10 +188,11 @@ func GetAndSaveUser(ctx context.Context, db database.DB, op GetAndSaveUserOp) (n ctx, db, usagestats.Event{ - EventName: eventName, - UserID: act.UID, - Argument: args, - Source: "BACKEND", + EventName: eventName, + UserID: act.UID, + Argument: args, + PublicArgument: args, + Source: "BACKEND", }, ) if err != nil { diff --git a/cmd/frontend/internal/auth/azureoauth/session.go b/cmd/frontend/internal/auth/azureoauth/session.go index 2ad0bbfa5a3..d93917f6486 100644 --- a/cmd/frontend/internal/auth/azureoauth/session.go +++ b/cmd/frontend/internal/auth/azureoauth/session.go @@ -109,6 +109,10 @@ func (s *sessionIssuerHelper) AuthFailedEventName() database.SecurityEventName { return database.SecurityEventAzureDevOpsAuthFailed } +func (s *sessionIssuerHelper) GetServiceID() string { + return s.ServiceID +} + func (s *sessionIssuerHelper) verifyAllowOrgs(ctx context.Context, profile *azuredevops.Profile, token *oauth2.Token) (bool, error) { if len(s.allowOrgs) == 0 { return true, nil diff --git a/cmd/frontend/internal/auth/bitbucketcloudoauth/session.go b/cmd/frontend/internal/auth/bitbucketcloudoauth/session.go index f18bd70c0ec..1292b55dc9d 100644 --- a/cmd/frontend/internal/auth/bitbucketcloudoauth/session.go +++ b/cmd/frontend/internal/auth/bitbucketcloudoauth/session.go @@ -39,6 +39,10 @@ func (s *sessionIssuerHelper) AuthFailedEventName() database.SecurityEventName { return database.SecurityEventBitbucketCloudAuthFailed } +func (s *sessionIssuerHelper) GetServiceID() string { + return s.baseURL.String() +} + func (s *sessionIssuerHelper) GetOrCreateUser(ctx context.Context, token *oauth2.Token, anonymousUserID, firstSourceURL, lastSourceURL string) (newUserCreated bool, actr *actor.Actor, safeErrMsg string, err error) { var client bitbucketcloud.Client if s.client != nil { diff --git a/cmd/frontend/internal/auth/githuboauth/session.go b/cmd/frontend/internal/auth/githuboauth/session.go index 6225448e640..e7eaa80aa91 100644 --- a/cmd/frontend/internal/auth/githuboauth/session.go +++ b/cmd/frontend/internal/auth/githuboauth/session.go @@ -46,6 +46,10 @@ func (s *sessionIssuerHelper) AuthFailedEventName() database.SecurityEventName { return database.SecurityEventGitHubAuthFailed } +func (s *sessionIssuerHelper) GetServiceID() string { + return s.ServiceID +} + func (s *sessionIssuerHelper) GetOrCreateUser(ctx context.Context, token *oauth2.Token, anonymousUserID, firstSourceURL, lastSourceURL string) (newUserCreated bool, actr *actor.Actor, safeErrMsg string, err error) { ghUser, err := github.UserFromContext(ctx) diff --git a/cmd/frontend/internal/auth/gitlaboauth/session.go b/cmd/frontend/internal/auth/gitlaboauth/session.go index e4e5112f1f5..11f2ec1263a 100644 --- a/cmd/frontend/internal/auth/gitlaboauth/session.go +++ b/cmd/frontend/internal/auth/gitlaboauth/session.go @@ -38,6 +38,10 @@ func (s *sessionIssuerHelper) AuthFailedEventName() database.SecurityEventName { return database.SecurityEventGitLabAuthFailed } +func (s *sessionIssuerHelper) GetServiceID() string { + return s.ServiceID +} + func (s *sessionIssuerHelper) GetOrCreateUser(ctx context.Context, token *oauth2.Token, anonymousUserID, firstSourceURL, lastSourceURL string) (newUserCreated bool, actr *actor.Actor, safeErrMsg string, err error) { gUser, err := UserFromContext(ctx) if err != nil { diff --git a/cmd/frontend/internal/auth/httpheader/middleware.go b/cmd/frontend/internal/auth/httpheader/middleware.go index a2ee11388ce..1a37ec8b9c3 100644 --- a/cmd/frontend/internal/auth/httpheader/middleware.go +++ b/cmd/frontend/internal/auth/httpheader/middleware.go @@ -119,7 +119,7 @@ func middleware(db database.DB) func(next http.Handler) http.Handler { return } - auth.AddPostAuthRedirectParametersToURL(r.URL, newUserCreated) + auth.AddPostAuthRedirectParametersToURL(r.URL, newUserCreated, "http-header") r = r.WithContext(actor.WithActor(r.Context(), &actor.Actor{UID: userID})) next.ServeHTTP(w, r) }) diff --git a/cmd/frontend/internal/auth/oauth/session.go b/cmd/frontend/internal/auth/oauth/session.go index 93a61935ef1..7d2e7fc6049 100644 --- a/cmd/frontend/internal/auth/oauth/session.go +++ b/cmd/frontend/internal/auth/oauth/session.go @@ -35,6 +35,7 @@ type SessionIssuerHelper interface { SessionData(token *oauth2.Token) SessionData AuthSucceededEventName() database.SecurityEventName AuthFailedEventName() database.SecurityEventName + GetServiceID() string } func SessionIssuer(logger log.Logger, db database.DB, s SessionIssuerHelper, sessionKey string) http.Handler { @@ -145,7 +146,7 @@ func SessionIssuer(logger log.Logger, db database.DB, s SessionIssuerHelper, ses logger.Warn("Failed to set OAuth session data. The session is still secure, but Sourcegraph will be unable to revoke the user's token or redirect the user to the end-session endpoint after the user signs out of Sourcegraph.", log.Error(err)) } - redirectURL := auth.AddPostAuthRedirectParametersToString(state.Redirect, newUserCreated) + redirectURL := auth.AddPostAuthRedirectParametersToString(state.Redirect, newUserCreated, "OAuth::"+s.GetServiceID()) http.Redirect(w, r, auth.SafeRedirectURL(redirectURL), http.StatusFound) }) } diff --git a/cmd/frontend/internal/auth/openidconnect/middleware.go b/cmd/frontend/internal/auth/openidconnect/middleware.go index 35a0bbd253c..43eff145801 100644 --- a/cmd/frontend/internal/auth/openidconnect/middleware.go +++ b/cmd/frontend/internal/auth/openidconnect/middleware.go @@ -345,7 +345,7 @@ func AuthCallback(db database.DB, r *http.Request, stateCookieName, usernamePref } // Add a ?signup= or ?signin= parameter to the redirect URL. - redirectURL := auth.AddPostAuthRedirectParametersToString(state.Redirect, newUserCreated) + redirectURL := auth.AddPostAuthRedirectParametersToString(state.Redirect, newUserCreated, "OpenIDConnect") user, err := db.Users().GetByID(r.Context(), actor.UID) if err != nil { diff --git a/cmd/frontend/internal/auth/openidconnect/middleware_test.go b/cmd/frontend/internal/auth/openidconnect/middleware_test.go index 1e7b45a3cba..99e3f0dd9a1 100644 --- a/cmd/frontend/internal/auth/openidconnect/middleware_test.go +++ b/cmd/frontend/internal/auth/openidconnect/middleware_test.go @@ -293,7 +293,7 @@ func TestMiddleware(t *testing.T) { if want := http.StatusFound; resp.StatusCode != want { t.Errorf("got status code %v, want %v", resp.StatusCode, want) } - if got, want := resp.Header.Get("Location"), "/redirect?signin="; got != want { + if got, want := resp.Header.Get("Location"), "/redirect?signin=OpenIDConnect"; got != want { t.Errorf("got redirect URL %v, want %v", got, want) } }) diff --git a/cmd/frontend/internal/auth/saml/middleware.go b/cmd/frontend/internal/auth/saml/middleware.go index b850ec2765e..81fa53fd208 100644 --- a/cmd/frontend/internal/auth/saml/middleware.go +++ b/cmd/frontend/internal/auth/saml/middleware.go @@ -181,7 +181,7 @@ func samlSPHandler(db database.DB) func(w http.ResponseWriter, r *http.Request) } // Add a ?signup= or ?signin= parameter to the redirect URL. - redirectURL := auth.AddPostAuthRedirectParametersToString(relayState.ReturnToURL, newUserCreated) + redirectURL := auth.AddPostAuthRedirectParametersToString(relayState.ReturnToURL, newUserCreated, "SAML") // 🚨 SECURITY: Call auth.SafeRedirectURL to avoid an open-redirect vuln. http.Redirect(w, r, auth.SafeRedirectURL(redirectURL), http.StatusFound) diff --git a/cmd/frontend/internal/auth/saml/middleware_test.go b/cmd/frontend/internal/auth/saml/middleware_test.go index 0069b2b5dc1..42fd006f2bb 100644 --- a/cmd/frontend/internal/auth/saml/middleware_test.go +++ b/cmd/frontend/internal/auth/saml/middleware_test.go @@ -374,7 +374,7 @@ func TestMiddleware(t *testing.T) { if want := http.StatusFound; resp.StatusCode != want { t.Errorf("got status code %v, want %v", resp.StatusCode, want) } - if got, want1, want2 := resp.Header.Get("Location"), "http://example.com/?signin=", "/?signin="; got != want1 && got != want2 { + if got, want1, want2 := resp.Header.Get("Location"), "http://example.com/?signin=SAML", "/?signin=SAML"; got != want1 && got != want2 { t.Errorf("got redirect location %v, want %v or %v", got, want1, want2) } diff --git a/cmd/frontend/internal/auth/sourcegraphoperator/middleware_test.go b/cmd/frontend/internal/auth/sourcegraphoperator/middleware_test.go index 32104c2f172..d5a9ffdbe31 100644 --- a/cmd/frontend/internal/auth/sourcegraphoperator/middleware_test.go +++ b/cmd/frontend/internal/auth/sourcegraphoperator/middleware_test.go @@ -270,7 +270,7 @@ func TestMiddleware(t *testing.T) { } resp := mocks.doRequest(http.MethodGet, urlStr, "", cookies, false) assert.Equal(t, http.StatusFound, resp.StatusCode) - wantRedirect := fmt.Sprintf(`%s?signin=`, state.Redirect) + wantRedirect := fmt.Sprintf(`%s?signin=OpenIDConnect`, state.Redirect) assert.Equal(t, wantRedirect, resp.Header.Get("Location")) mockrequire.CalledOnce(t, mocks.usersStore.SetIsSiteAdminFunc) })