[bugfix] Fix '+'-separated scopes not being recognized (#4028)

* [bugfix] Fix '+'-separated scopes not being recognized

* comment

6 files changed, 35 insertions, 19 deletions

diff --git a/internal/api/auth/authorize.go b/internal/api/auth/authorize.go
index 3676fd417..5b3e6ea3c 100644
--- a/internal/api/auth/authorize.go
+++ b/internal/api/auth/authorize.go
@@ -20,6 +20,7 @@ package auth
 import (
 	"net/http"
 	"net/url"
+	"strings"
 
 	"github.com/gin-contrib/sessions"
 	"github.com/gin-gonic/gin"
@@ -229,8 +230,8 @@ func (m *Module) AuthorizePOSTHandler(c *gin.Context) {
 }
 
 // redirectAuthFormToSignIn binds an OAuthAuthorize form,
-// stores the values in the form into the session, and
-// redirects the user to the sign in page.
+// presumed to be set as url query params, stores the values
+// into the session, and redirects the user to the sign in page.
 func (m *Module) redirectAuthFormToSignIn(c *gin.Context) {
 	s := sessions.Default(c)
 
@@ -240,9 +241,14 @@ func (m *Module) redirectAuthFormToSignIn(c *gin.Context) {
 		return
 	}
 
-	// Set default scope to read.
+	// If scope isn't set default to read.
+	//
+	// Else massage submitted scope(s) from
+	// '+'-separated to space-separated.
 	if form.Scope == "" {
 		form.Scope = "read"
+	} else {
+		form.Scope = strings.ReplaceAll(form.Scope, "+", " ")
 	}
 
 	// Save these values from the form so we
diff --git a/internal/api/auth/revoke.go b/internal/api/auth/revoke.go
index bb621e5e0..7bd565308 100644
--- a/internal/api/auth/revoke.go
+++ b/internal/api/auth/revoke.go
@@ -80,10 +80,12 @@ func (m *Module) TokenRevokePOSTHandler(c *gin.Context) {
 		return
 	}
 
+	// Don't set `binding:"required"` on these
+	// fields as we want to validate them ourself.
 	form := &struct {
-		ClientID     string `form:"client_id" validate:"required"`
-		ClientSecret string `form:"client_secret" validate:"required"`
-		Token        string `form:"token" validate:"required"`
+		ClientID     string `form:"client_id"`
+		ClientSecret string `form:"client_secret"`
+		Token        string `form:"token"`
 	}{}
 	if err := c.ShouldBind(form); err != nil {
 		errWithCode := gtserror.NewErrorBadRequest(err, err.Error())
diff --git a/internal/api/auth/signin.go b/internal/api/auth/signin.go
index 2820255db..3503f37bb 100644
--- a/internal/api/auth/signin.go
+++ b/internal/api/auth/signin.go
@@ -101,8 +101,8 @@ func (m *Module) SignInPOSTHandler(c *gin.Context) {
 
 	// Parse email + password.
 	form := &struct {
-		Email    string `form:"username" validate:"required"`
-		Password string `form:"password" validate:"required"`
+		Email    string `form:"username" binding:"required"`
+		Password string `form:"password" binding:"required"`
 	}{}
 	if err := c.ShouldBind(form); err != nil {
 		m.clearSessionWithBadRequest(c, s, err, oauth.HelpfulAdvice)
@@ -235,7 +235,7 @@ func (m *Module) TwoFactorCodePOSTHandler(c *gin.Context) {
 
 	// Parse 2fa code.
 	form := &struct {
-		Code string `form:"code" validate:"required"`
+		Code string `form:"code" binding:"required"`
 	}{}
 	if err := c.ShouldBind(form); err != nil {
 		m.clearSessionWithBadRequest(c, s, err, oauth.HelpfulAdvice)
diff --git a/internal/api/model/oauth.go b/internal/api/model/oauth.go
index a4840b10a..8e903761e 100644
--- a/internal/api/model/oauth.go
+++ b/internal/api/model/oauth.go
@@ -22,13 +22,13 @@ type OAuthAuthorize struct {
 	// Forces the user to re-login, which is necessary for authorizing with multiple accounts from the same instance.
 	ForceLogin string `form:"force_login" json:"force_login"`
 	// Should be set equal to `code`.
-	ResponseType string `form:"response_type" json:"response_type" validate:"required"`
+	ResponseType string `form:"response_type" json:"response_type" binding:"required"`
 	// Client ID, obtained during app registration.
-	ClientID string `form:"client_id" json:"client_id" validate:"required"`
+	ClientID string `form:"client_id" json:"client_id" binding:"required"`
 	// Set a URI to redirect the user to.
 	// If this parameter is set to urn:ietf:wg:oauth:2.0:oob then the authorization code will be shown instead.
 	// Must match one of the redirect URIs declared during app registration.
-	RedirectURI string `form:"redirect_uri" json:"redirect_uri" validate:"required"`
+	RedirectURI string `form:"redirect_uri" json:"redirect_uri" binding:"required"`
 	// List of requested OAuth scopes, separated by spaces (or by pluses, if using query parameters).
 	// Must be a subset of scopes declared during app registration. If not provided, defaults to read.
 	Scope string `form:"scope" json:"scope"`
diff --git a/web/source/settings/lib/query/login/index.ts b/web/source/settings/lib/query/login/index.ts
index dc85e9efd..1f56a51c5 100644
--- a/web/source/settings/lib/query/login/index.ts
+++ b/web/source/settings/lib/query/login/index.ts
@@ -28,6 +28,7 @@ import {
 import { RootState } from '../../../redux/store';
 import { Account } from '../../types/account';
 import { OAuthAccessTokenRequestBody } from '../../types/oauth';
+import { App } from '../../types/application';
 
 function getSettingsURL() {
 	/*
@@ -129,7 +130,7 @@ const extended = gtsApi.injectEndpoints({
 			}
 		}),
 
-		authorizeFlow: build.mutation({
+		authorizeFlow: build.mutation<any, { instance: string, scopes: string }>({
 			async queryFn(formData, api, _extraOpts, fetchWithBQ) {
 				const state = api.getState() as RootState;
 				const loginState = state.login;
@@ -159,22 +160,26 @@ const extended = gtsApi.injectEndpoints({
 					return { error: appResult.error as FetchBaseQueryError };
 				}
 
-				const app = appResult.data as any;
-
-				app.scopes = formData.scopes;
+				const app = appResult.data as App;
 				api.dispatch(oauthAuthorize({
 					instanceUrl: instanceUrl,
-					app: app,
+					app: {
+						client_id: app.client_id,
+						client_secret: app.client_secret,
+					},
 					current: "awaitingcallback",
 					expectingRedirect: true
 				}));
 
+				// Parse instance URL + set params on it.
+				//
+				// Note that scopes are '+'-separated to fit the API.
 				const url = new URL(instanceUrl);
 				url.pathname = "/oauth/authorize";
 				url.searchParams.set("client_id", app.client_id);
 				url.searchParams.set("redirect_uri", SETTINGS_URL);
 				url.searchParams.set("response_type", "code");
-				url.searchParams.set("scope", app.scopes);
+				url.searchParams.set("scope", app.scopes.join("+"));
 				
 				const redirectURL = url.toString();
 				window.location.assign(redirectURL);
diff --git a/web/source/settings/lib/query/user/applications.ts b/web/source/settings/lib/query/user/applications.ts
index 9d271a1e1..38856ccba 100644
--- a/web/source/settings/lib/query/user/applications.ts
+++ b/web/source/settings/lib/query/user/applications.ts
@@ -107,12 +107,15 @@ const extended = gtsApi.injectEndpoints({
 				const instanceUrl = state.login.instanceUrl;
 
 				// Parse instance URL + set params on it.
+				//
+				// Note that any space-separated scopes are
+				// replaced by '+'-separated, to fit the API.
 				const url = new URL(instanceUrl);
 				url.pathname = "/oauth/authorize";
 				url.searchParams.set("client_id", app.client_id);
 				url.searchParams.set("redirect_uri", redirectURI);
 				url.searchParams.set("response_type", "code");
-				url.searchParams.set("scope", scope);
+				url.searchParams.set("scope", scope.replace(" ", "+"));
 
 				// Set the app ID in state so we know which
 				// app to get out of our store after redirect.