Skip to content

Commit 72738f0

Browse files
authored
Lock goth/gothic and Re-attempt OAuth2 registration on login if registration failed at startup (#16564)
This PR has two parts: * Add locking to goth and gothic calls with a RWMutex The goth and gothic calls are currently unlocked and thus are a cause of multiple potential races * Reattempt OAuth2 registration on login if registration failed If OAuth2 registration fails at startup we currently disable the login_source however an alternative approach could be to reattempt registration on login attempt. Fix #16096 Signed-off-by: Andrew Thornton <art27@cantab.net>
1 parent b9a0e33 commit 72738f0

File tree

3 files changed

+26
-6
lines changed

3 files changed

+26
-6
lines changed

services/auth/source/oauth2/init.go

+11-6
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package oauth2
66

77
import (
88
"net/http"
9+
"sync"
910

1011
"code.gitea.io/gitea/models"
1112
"code.gitea.io/gitea/modules/log"
@@ -15,6 +16,8 @@ import (
1516
"github.com/markbates/goth/gothic"
1617
)
1718

19+
var gothRWMutex = sync.RWMutex{}
20+
1821
// SessionTableName is the table name that OAuth2 will use to store things
1922
const SessionTableName = "oauth2_session"
2023

@@ -42,6 +45,10 @@ func Init() error {
4245

4346
// Note, when using the FilesystemStore only the session.ID is written to a browser cookie, so this is explicit for the storage on disk
4447
store.MaxLength(setting.OAuth2.MaxTokenLength)
48+
49+
// Lock our mutex
50+
gothRWMutex.Lock()
51+
4552
gothic.Store = store
4653

4754
gothic.SetState = func(req *http.Request) string {
@@ -52,6 +59,9 @@ func Init() error {
5259
return req.Header.Get(ProviderHeaderKey), nil
5360
}
5461

62+
// Unlock our mutex
63+
gothRWMutex.Unlock()
64+
5565
return initOAuth2LoginSources()
5666
}
5767

@@ -71,12 +81,7 @@ func initOAuth2LoginSources() error {
7181
}
7282
err := oauth2Source.RegisterSource()
7383
if err != nil {
74-
log.Critical("Unable to register source: %s due to Error: %v. This source will be disabled.", source.Name, err)
75-
source.IsActive = false
76-
if err = models.UpdateSource(source); err != nil {
77-
log.Critical("Unable to update source %s to disable it. Error: %v", err)
78-
return err
79-
}
84+
log.Critical("Unable to register source: %s due to Error: %v.", source.Name, err)
8085
}
8186
}
8287
return nil

services/auth/source/oauth2/providers.go

+9
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ func RegisterProvider(providerName, providerType, clientID, clientSecret, openID
121121
provider, err := createProvider(providerName, providerType, clientID, clientSecret, openIDConnectAutoDiscoveryURL, customURLMapping)
122122

123123
if err == nil && provider != nil {
124+
gothRWMutex.Lock()
125+
defer gothRWMutex.Unlock()
126+
124127
goth.UseProviders(provider)
125128
}
126129

@@ -129,11 +132,17 @@ func RegisterProvider(providerName, providerType, clientID, clientSecret, openID
129132

130133
// RemoveProvider removes the given OAuth2 provider from the goth lib
131134
func RemoveProvider(providerName string) {
135+
gothRWMutex.Lock()
136+
defer gothRWMutex.Unlock()
137+
132138
delete(goth.GetProviders(), providerName)
133139
}
134140

135141
// ClearProviders clears all OAuth2 providers from the goth lib
136142
func ClearProviders() {
143+
gothRWMutex.Lock()
144+
defer gothRWMutex.Unlock()
145+
137146
goth.ClearProviders()
138147
}
139148

services/auth/source/oauth2/source_callout.go

+6
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ func (source *Source) Callout(request *http.Request, response http.ResponseWrite
2020
// normally the gothic library will write some custom stuff to the response instead of our own nice error page
2121
//gothic.BeginAuthHandler(response, request)
2222

23+
gothRWMutex.RLock()
24+
defer gothRWMutex.RUnlock()
25+
2326
url, err := gothic.GetAuthURL(response, request)
2427
if err == nil {
2528
http.Redirect(response, request, url, http.StatusTemporaryRedirect)
@@ -33,6 +36,9 @@ func (source *Source) Callback(request *http.Request, response http.ResponseWrit
3336
// not sure if goth is thread safe (?) when using multiple providers
3437
request.Header.Set(ProviderHeaderKey, source.loginSource.Name)
3538

39+
gothRWMutex.RLock()
40+
defer gothRWMutex.RUnlock()
41+
3642
user, err := gothic.CompleteUserAuth(response, request)
3743
if err != nil {
3844
return user, err

0 commit comments

Comments
 (0)