diff --git a/app/handlers/oauth.go b/app/handlers/oauth.go index 2a8672097..0ffdf0616 100644 --- a/app/handlers/oauth.go +++ b/app/handlers/oauth.go @@ -162,7 +162,14 @@ func OAuthCallback() web.HandlerFunc { c.Response.Header().Add("X-Robots-Tag", "noindex") provider := c.Param("provider") + + // Support both query parameters (GET) and form data (POST) + // Apple Sign-In uses POST with response_mode=form_post when name/email scopes are requested state := c.QueryParam("state") + if state == "" && c.Request.Method == "POST" { + state = c.Request.GetFormValue("state") + } + claims, err := jwt.DecodeOAuthStateClaims(state) if err != nil { return c.Forbidden() @@ -179,6 +186,9 @@ func OAuthCallback() web.HandlerFunc { } code := c.QueryParam("code") + if code == "" && c.Request.Method == "POST" { + code = c.Request.GetFormValue("code") + } if code == "" { return c.Redirect(redirectURL.String()) } diff --git a/app/handlers/oauth_test.go b/app/handlers/oauth_test.go index 969abc25d..de71f5cff 100644 --- a/app/handlers/oauth_test.go +++ b/app/handlers/oauth_test.go @@ -280,6 +280,30 @@ func TestCallbackHandler_SignUp(t *testing.T) { }) } +func TestCallbackHandler_SignIn_FormPost(t *testing.T) { + RegisterT(t) + + state, _ := jwt.Encode(jwt.OAuthStateClaims{ + Redirect: "http://avengers.test.fider.io", + Identifier: "888", + }) + + // Simulate Apple Sign-In with form_post response mode + values := url.Values{} + values.Set("state", state) + values.Set("code", "123") + formData := values.Encode() + + server := mock.NewServer() + code, response := server. + WithURL("http://login.test.fider.io/oauth/callback"). + AddParam("provider", "_apple"). + ExecutePostForm(handlers.OAuthCallback(), formData) + + Expect(code).Equals(http.StatusTemporaryRedirect) + Expect(response.Header().Get("Location")).Equals("http://avengers.test.fider.io/oauth/_apple/token?code=123&identifier=888&redirect=%2F") +} + func TestOAuthTokenHandler_ExistingUserAndProvider(t *testing.T) { RegisterT(t) diff --git a/app/pkg/mock/server.go b/app/pkg/mock/server.go index 331c62bac..7f87d95ef 100644 --- a/app/pkg/mock/server.go +++ b/app/pkg/mock/server.go @@ -132,6 +132,38 @@ func (s *Server) ExecutePost(handler web.HandlerFunc, body string) (int, *httpte return s.Execute(handler) } +// ExecutePostForm executes given handler as POST with form data and return response +func (s *Server) ExecutePostForm(handler web.HandlerFunc, body string) (int, *httptest.ResponseRecorder) { + // Create a new HTTP request with the body as a reader for FormValue to work + req := httptest.NewRequest("POST", s.context.Request.URL.String(), strings.NewReader(body)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + // Create new context with proper request (empty params initially) + params := make(web.StringMap) + newContext := web.NewContext(s.engine, req, s.recorder, params) + + // Copy params manually by using the Param method on old context + // Note: This only copies known params. If you need to copy all params, + // consider extending the Context API to expose all params. + if providerParam := s.context.Param("provider"); providerParam != "" { + newContext.AddParam("provider", providerParam) + } + + // Copy tenant and user from old context if they exist + if s.context.Tenant() != nil { + newContext.SetTenant(s.context.Tenant()) + } + if s.context.User() != nil { + newContext.SetUser(s.context.User()) + } + + // Replace the context + s.context = newContext + + // Execute with the handler + return s.Execute(handler) +} + // ExecutePostAsJSON executes given handler as POST and return json response func (s *Server) ExecutePostAsJSON(handler web.HandlerFunc, body string) (int, *jsonq.Query) { code, response := s.ExecutePost(handler, body) diff --git a/app/pkg/web/request.go b/app/pkg/web/request.go index 39a9c1c4f..848b942c9 100644 --- a/app/pkg/web/request.go +++ b/app/pkg/web/request.go @@ -1,6 +1,7 @@ package web import ( + "bytes" "io" "net/http" "net/url" @@ -47,6 +48,9 @@ func WrapRequest(request *http.Request) Request { if err != nil { panic(errors.Wrap(err, "failed to read body").Error()) } + // Reset the body so it can be read again by FormValue for form-encoded data + // or other handlers that may need to parse the request body + request.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) } return Request{ @@ -79,6 +83,11 @@ func (r *Request) Cookie(name string) (*http.Cookie, error) { return cookie, nil } +// GetFormValue returns a form value from POST request +func (r *Request) GetFormValue(key string) string { + return r.instance.FormValue(key) +} + // AddCookie adds a cookie func (r *Request) AddCookie(cookie *http.Cookie) { r.instance.AddCookie(cookie) diff --git a/app/services/oauth/custom.go b/app/services/oauth/custom.go index 079c57fb0..26a49ccb4 100644 --- a/app/services/oauth/custom.go +++ b/app/services/oauth/custom.go @@ -1,6 +1,11 @@ package oauth -import "net/url" +import ( + "net/url" + "strings" + + "github.com/getfider/fider/app/models/entity" +) var providerParams = map[string]map[string]string{ "id.twitch.tv": { @@ -8,12 +13,25 @@ var providerParams = map[string]map[string]string{ }, } -func getProviderInitialParams(u *url.URL) url.Values { +func getProviderInitialParams(u *url.URL, config *entity.OAuthConfig) url.Values { v := url.Values{} - if params, ok := providerParams[u.Hostname()]; ok { + hostname := u.Hostname() + + // Add static provider-specific parameters + if params, ok := providerParams[hostname]; ok { for key, value := range params { v.Add(key, value) } } + + // Add dynamic parameters based on provider and scope + // Apple Sign-In requires response_mode=form_post when name or email scopes are requested + if hostname == "appleid.apple.com" && config != nil { + scope := strings.ToLower(config.Scope) + if strings.Contains(scope, "name") || strings.Contains(scope, "email") { + v.Add("response_mode", "form_post") + } + } + return v } diff --git a/app/services/oauth/oauth.go b/app/services/oauth/oauth.go index 718945197..601457c52 100644 --- a/app/services/oauth/oauth.go +++ b/app/services/oauth/oauth.go @@ -201,7 +201,7 @@ func getOAuthAuthorizationURL(ctx context.Context, q *query.GetOAuthAuthorizatio oauthBaseURL := web.OAuthBaseURL(ctx) authURL, _ := url.Parse(config.AuthorizeURL) - parameters := getProviderInitialParams(authURL) + parameters := getProviderInitialParams(authURL, config) parameters.Add("client_id", config.ClientID) parameters.Add("scope", config.Scope) parameters.Add("redirect_uri", fmt.Sprintf("%s/oauth/%s/callback", oauthBaseURL, q.Provider)) diff --git a/app/services/oauth/oauth_test.go b/app/services/oauth/oauth_test.go index 16b018846..b48ab5fb3 100644 --- a/app/services/oauth/oauth_test.go +++ b/app/services/oauth/oauth_test.go @@ -194,6 +194,107 @@ func TestGetAuthURL_Twitch(t *testing.T) { Expect(authURL.Result).Equals("https://id.twitch.tv/oauth/authorize?claims=%7B%22userinfo%22%3A%7B%22preferred_username%22%3Anull%2C%22email%22%3Anull%2C%22email_verified%22%3Anull%7D%7D&client_id=CU_CL_ID&redirect_uri=http%3A%2F%2Flogin.test.fider.io%3A3000%2Foauth%2F_custom%2Fcallback&response_type=code&scope=openid&state=" + expectedState) } +func TestGetAuthURL_Apple_WithEmailScope(t *testing.T) { + RegisterT(t) + bus.Init(&oauth.Service{}) + + bus.AddHandler(func(ctx context.Context, q *query.GetCustomOAuthConfigByProvider) error { + if q.Provider == "_apple" { + q.Result = &entity.OAuthConfig{ + Provider: q.Provider, + ClientID: "APPLE_CL_ID", + Scope: "openid email", + AuthorizeURL: "https://appleid.apple.com/auth/authorize", + } + } + return nil + }) + + ctx := newGetContext("http://login.test.fider.io:3000") + authURL := &query.GetOAuthAuthorizationURL{ + Provider: "_apple", + Redirect: "http://example.org", + Identifier: "123", + } + + expectedState, _ := jwt.Encode(jwt.OAuthStateClaims{ + Redirect: "http://example.org", + Identifier: "123", + }) + + err := bus.Dispatch(ctx, authURL) + Expect(err).IsNil() + Expect(authURL.Result).Equals("https://appleid.apple.com/auth/authorize?client_id=APPLE_CL_ID&redirect_uri=http%3A%2F%2Flogin.test.fider.io%3A3000%2Foauth%2F_apple%2Fcallback&response_mode=form_post&response_type=code&scope=openid+email&state=" + expectedState) +} + +func TestGetAuthURL_Apple_WithNameScope(t *testing.T) { + RegisterT(t) + bus.Init(&oauth.Service{}) + + bus.AddHandler(func(ctx context.Context, q *query.GetCustomOAuthConfigByProvider) error { + if q.Provider == "_apple" { + q.Result = &entity.OAuthConfig{ + Provider: q.Provider, + ClientID: "APPLE_CL_ID", + Scope: "openid name", + AuthorizeURL: "https://appleid.apple.com/auth/authorize", + } + } + return nil + }) + + ctx := newGetContext("http://login.test.fider.io:3000") + authURL := &query.GetOAuthAuthorizationURL{ + Provider: "_apple", + Redirect: "http://example.org", + Identifier: "456", + } + + expectedState, _ := jwt.Encode(jwt.OAuthStateClaims{ + Redirect: "http://example.org", + Identifier: "456", + }) + + err := bus.Dispatch(ctx, authURL) + Expect(err).IsNil() + Expect(authURL.Result).Equals("https://appleid.apple.com/auth/authorize?client_id=APPLE_CL_ID&redirect_uri=http%3A%2F%2Flogin.test.fider.io%3A3000%2Foauth%2F_apple%2Fcallback&response_mode=form_post&response_type=code&scope=openid+name&state=" + expectedState) +} + +func TestGetAuthURL_Apple_WithoutEmailOrName(t *testing.T) { + RegisterT(t) + bus.Init(&oauth.Service{}) + + bus.AddHandler(func(ctx context.Context, q *query.GetCustomOAuthConfigByProvider) error { + if q.Provider == "_apple" { + q.Result = &entity.OAuthConfig{ + Provider: q.Provider, + ClientID: "APPLE_CL_ID", + Scope: "openid", + AuthorizeURL: "https://appleid.apple.com/auth/authorize", + } + } + return nil + }) + + ctx := newGetContext("http://login.test.fider.io:3000") + authURL := &query.GetOAuthAuthorizationURL{ + Provider: "_apple", + Redirect: "http://example.org", + Identifier: "789", + } + + expectedState, _ := jwt.Encode(jwt.OAuthStateClaims{ + Redirect: "http://example.org", + Identifier: "789", + }) + + err := bus.Dispatch(ctx, authURL) + Expect(err).IsNil() + // Without email or name scope, response_mode should not be added + Expect(authURL.Result).Equals("https://appleid.apple.com/auth/authorize?client_id=APPLE_CL_ID&redirect_uri=http%3A%2F%2Flogin.test.fider.io%3A3000%2Foauth%2F_apple%2Fcallback&response_type=code&scope=openid&state=" + expectedState) +} + + func TestParseProfileResponse_AllFields(t *testing.T) { RegisterT(t) bus.Init(&oauth.Service{}) diff --git a/go.mod b/go.mod index bdc3e3d6b..930fbfe15 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/lib/pq v1.10.9 github.com/microcosm-cc/bluemonday v1.0.16 github.com/patrickmn/go-cache v2.1.0+incompatible + github.com/pemistahl/lingua-go v1.4.0 github.com/prometheus/client_golang v1.12.1 github.com/prometheus/client_model v0.2.0 github.com/robfig/cron v1.2.0 @@ -150,7 +151,6 @@ require ( github.com/olekukonko/tablewriter v0.0.5 // indirect github.com/pelletier/go-toml v1.9.5 // indirect github.com/pelletier/go-toml/v2 v2.2.2 // indirect - github.com/pemistahl/lingua-go v1.4.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/polyfloyd/go-errorlint v1.5.2 // indirect github.com/prometheus/common v0.32.1 // indirect