Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions app/handlers/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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())
}
Expand Down
24 changes: 24 additions & 0 deletions app/handlers/oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
32 changes: 32 additions & 0 deletions app/pkg/mock/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions app/pkg/web/request.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package web

import (
"bytes"
"io"
"net/http"
"net/url"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)
Expand Down
24 changes: 21 additions & 3 deletions app/services/oauth/custom.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,37 @@
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": {
"claims": `{"userinfo":{"preferred_username":null,"email":null,"email_verified":null}}`,
},
}

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")
}
}
Comment on lines +29 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something that will be required from other oauth providers in the future, and if so should we make it configurable, or is it unlikey?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach is designed to be extensible. Currently, Apple is the only known provider requiring dynamic parameters based on scope. The implementation handles both:

  1. Static parameters via the providerParams map (like Twitch's claims parameter)
  2. Dynamic parameters based on config (like Apple's response_mode)

If other providers need similar dynamic behavior in the future, the pattern is already established - just add another hostname check with the appropriate logic. Making it fully configurable (e.g., via database or config file) would add complexity without a clear use case yet.


return v
}
2 changes: 1 addition & 1 deletion app/services/oauth/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
101 changes: 101 additions & 0 deletions app/services/oauth/oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading