mirror of
				https://github.com/go-gitea/gitea
				synced 2025-10-31 19:38:23 +00:00 
			
		
		
		
	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>
			
			
This commit is contained in:
		| @@ -6,6 +6,7 @@ package oauth2 | ||||
|  | ||||
| import ( | ||||
| 	"net/http" | ||||
| 	"sync" | ||||
|  | ||||
| 	"code.gitea.io/gitea/models" | ||||
| 	"code.gitea.io/gitea/modules/log" | ||||
| @@ -15,6 +16,8 @@ import ( | ||||
| 	"github.com/markbates/goth/gothic" | ||||
| ) | ||||
|  | ||||
| var gothRWMutex = sync.RWMutex{} | ||||
|  | ||||
| // SessionTableName is the table name that OAuth2 will use to store things | ||||
| const SessionTableName = "oauth2_session" | ||||
|  | ||||
| @@ -42,6 +45,10 @@ func Init() error { | ||||
|  | ||||
| 	// Note, when using the FilesystemStore only the session.ID is written to a browser cookie, so this is explicit for the storage on disk | ||||
| 	store.MaxLength(setting.OAuth2.MaxTokenLength) | ||||
|  | ||||
| 	// Lock our mutex | ||||
| 	gothRWMutex.Lock() | ||||
|  | ||||
| 	gothic.Store = store | ||||
|  | ||||
| 	gothic.SetState = func(req *http.Request) string { | ||||
| @@ -52,6 +59,9 @@ func Init() error { | ||||
| 		return req.Header.Get(ProviderHeaderKey), nil | ||||
| 	} | ||||
|  | ||||
| 	// Unlock our mutex | ||||
| 	gothRWMutex.Unlock() | ||||
|  | ||||
| 	return initOAuth2LoginSources() | ||||
| } | ||||
|  | ||||
| @@ -71,12 +81,7 @@ func initOAuth2LoginSources() error { | ||||
| 		} | ||||
| 		err := oauth2Source.RegisterSource() | ||||
| 		if err != nil { | ||||
| 			log.Critical("Unable to register source: %s due to Error: %v. This source will be disabled.", source.Name, err) | ||||
| 			source.IsActive = false | ||||
| 			if err = models.UpdateSource(source); err != nil { | ||||
| 				log.Critical("Unable to update source %s to disable it. Error: %v", err) | ||||
| 				return err | ||||
| 			} | ||||
| 			log.Critical("Unable to register source: %s due to Error: %v.", source.Name, err) | ||||
| 		} | ||||
| 	} | ||||
| 	return nil | ||||
|   | ||||
| @@ -121,6 +121,9 @@ func RegisterProvider(providerName, providerType, clientID, clientSecret, openID | ||||
| 	provider, err := createProvider(providerName, providerType, clientID, clientSecret, openIDConnectAutoDiscoveryURL, customURLMapping) | ||||
|  | ||||
| 	if err == nil && provider != nil { | ||||
| 		gothRWMutex.Lock() | ||||
| 		defer gothRWMutex.Unlock() | ||||
|  | ||||
| 		goth.UseProviders(provider) | ||||
| 	} | ||||
|  | ||||
| @@ -129,11 +132,17 @@ func RegisterProvider(providerName, providerType, clientID, clientSecret, openID | ||||
|  | ||||
| // RemoveProvider removes the given OAuth2 provider from the goth lib | ||||
| func RemoveProvider(providerName string) { | ||||
| 	gothRWMutex.Lock() | ||||
| 	defer gothRWMutex.Unlock() | ||||
|  | ||||
| 	delete(goth.GetProviders(), providerName) | ||||
| } | ||||
|  | ||||
| // ClearProviders clears all OAuth2 providers from the goth lib | ||||
| func ClearProviders() { | ||||
| 	gothRWMutex.Lock() | ||||
| 	defer gothRWMutex.Unlock() | ||||
|  | ||||
| 	goth.ClearProviders() | ||||
| } | ||||
|  | ||||
|   | ||||
| @@ -20,6 +20,9 @@ func (source *Source) Callout(request *http.Request, response http.ResponseWrite | ||||
| 	// normally the gothic library will write some custom stuff to the response instead of our own nice error page | ||||
| 	//gothic.BeginAuthHandler(response, request) | ||||
|  | ||||
| 	gothRWMutex.RLock() | ||||
| 	defer gothRWMutex.RUnlock() | ||||
|  | ||||
| 	url, err := gothic.GetAuthURL(response, request) | ||||
| 	if err == nil { | ||||
| 		http.Redirect(response, request, url, http.StatusTemporaryRedirect) | ||||
| @@ -33,6 +36,9 @@ func (source *Source) Callback(request *http.Request, response http.ResponseWrit | ||||
| 	// not sure if goth is thread safe (?) when using multiple providers | ||||
| 	request.Header.Set(ProviderHeaderKey, source.loginSource.Name) | ||||
|  | ||||
| 	gothRWMutex.RLock() | ||||
| 	defer gothRWMutex.RUnlock() | ||||
|  | ||||
| 	user, err := gothic.CompleteUserAuth(response, request) | ||||
| 	if err != nil { | ||||
| 		return user, err | ||||
|   | ||||
		Reference in New Issue
	
	Block a user