Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: trust custom oauth providers in order to create un-invited users in private mode #1068

Merged
merged 11 commits into from
May 14, 2022
4 changes: 3 additions & 1 deletion app/actions/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ package actions

import (
"context"
"github.com/getfider/fider/app"
"strings"

"github.com/getfider/fider/app"

"github.com/getfider/fider/app/models/dto"
"github.com/getfider/fider/app/models/entity"
"github.com/getfider/fider/app/models/enum"
Expand All @@ -28,6 +29,7 @@ type CreateEditOAuthConfig struct {
TokenURL string `json:"tokenURL"`
Scope string `json:"scope"`
ProfileURL string `json:"profileURL"`
IsTrusted bool `json:"isTrusted"`
JSONUserIDPath string `json:"jsonUserIDPath"`
JSONUserNamePath string `json:"jsonUserNamePath"`
JSONUserEmailPath string `json:"jsonUserEmailPath"`
Expand Down
3 changes: 2 additions & 1 deletion app/actions/oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ package actions_test

import (
"context"
"github.com/getfider/fider/app/models/dto"
"testing"

"github.com/getfider/fider/app/models/dto"

"github.com/getfider/fider/app"
"github.com/getfider/fider/app/actions"
"github.com/getfider/fider/app/models/entity"
Expand Down
1 change: 1 addition & 0 deletions app/handlers/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ func SaveOAuthConfig() web.HandlerFunc {
TokenURL: action.TokenURL,
Scope: action.Scope,
ProfileURL: action.ProfileURL,
IsTrusted: action.IsTrusted,
JSONUserIDPath: action.JSONUserIDPath,
JSONUserNamePath: action.JSONUserNamePath,
JSONUserEmailPath: action.JSONUserEmailPath,
Expand Down
13 changes: 12 additions & 1 deletion app/handlers/oauth.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package handlers

import (
"context"
"fmt"
"net/http"
"net/url"
Expand Down Expand Up @@ -102,7 +103,8 @@ func OAuthToken() web.HandlerFunc {
}
if err != nil {
if errors.Cause(err) == app.ErrNotFound {
if c.Tenant().IsPrivate {
isTrusted := isTrustedOAuthProvider(c, provider)
if c.Tenant().IsPrivate && !isTrusted {
return c.Redirect("/not-invited")
}

Expand Down Expand Up @@ -141,6 +143,15 @@ func OAuthToken() web.HandlerFunc {
}
}

func isTrustedOAuthProvider(ctx context.Context, provider string) bool {
customOAuthConfigByProvider := &query.GetCustomOAuthConfigByProvider{Provider: provider}
err := bus.Dispatch(ctx, customOAuthConfigByProvider)
if err != nil {
return false
}
return customOAuthConfigByProvider.Result.IsTrusted
}

// OAuthCallback handles the redirect back from the OAuth provider
// This callback can run on either Tenant or Login address
// If the request is for a sign in, we redirect the user to the tenant address
Expand Down
118 changes: 101 additions & 17 deletions app/handlers/oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/getfider/fider/app/models/dto"
"github.com/getfider/fider/app/models/entity"
"github.com/getfider/fider/app/models/enum"

"github.com/getfider/fider/app/services/oauth"

Expand Down Expand Up @@ -227,23 +228,31 @@ func TestOAuthTokenHandler_NewUser(t *testing.T) {
})

bus.AddHandler(func(ctx context.Context, q *query.GetUserByProvider) error {
Expect(q.Provider).Equals(app.FacebookProvider)
Expect(q.UID).Equals("FB456")
return app.ErrNotFound
})

bus.AddHandler(func(ctx context.Context, q *query.GetUserByEmail) error {
Expect(q.Email).Equals("some.guy@facebook.com")
return app.ErrNotFound
})

bus.AddHandler(func(ctx context.Context, q *query.GetCustomOAuthConfigByProvider) error {
Expect(q.Provider).Equals(app.FacebookProvider)
return app.ErrNotFound
})

bus.AddHandler(func(ctx context.Context, q *query.GetOAuthProfile) error {
if q.Provider == app.FacebookProvider && q.Code == "456" {
q.Result = &dto.OAuthUserProfile{
ID: "FB456",
Name: "Some Facebook Guy",
Email: "some.guy@facebook.com",
}
return nil
Expect(q.Provider).Equals(app.FacebookProvider)
Expect(q.Code).Equals("456")

q.Result = &dto.OAuthUserProfile{
ID: "FB456",
Name: "Some Facebook Guy",
Email: "some.guy@facebook.com",
}
return app.ErrNotFound
return nil
})

server := mock.NewServer()
Expand Down Expand Up @@ -278,6 +287,10 @@ func TestOAuthTokenHandler_NewUserWithoutEmail(t *testing.T) {
return app.ErrNotFound
})

bus.AddHandler(func(ctx context.Context, q *query.GetCustomOAuthConfigByProvider) error {
return app.ErrNotFound
})

bus.AddHandler(func(ctx context.Context, q *query.GetOAuthProfile) error {
if q.Provider == app.FacebookProvider && q.Code == "798" {
q.Result = &dto.OAuthUserProfile{
Expand Down Expand Up @@ -414,7 +427,7 @@ func TestOAuthTokenHandler_ExistingUser_NewProvider(t *testing.T) {
ExpectFiderAuthCookie(response, mock.JonSnow)
}

func TestOAuthTokenHandler_NewUser_PrivateTenant(t *testing.T) {
func TestOAuthTokenHandler_NewUser_PrivateSite(t *testing.T) {
RegisterT(t)

server := mock.NewServer()
Expand All @@ -428,16 +441,20 @@ func TestOAuthTokenHandler_NewUser_PrivateTenant(t *testing.T) {
return app.ErrNotFound
})

bus.AddHandler(func(ctx context.Context, q *query.GetCustomOAuthConfigByProvider) error {
Expect(q.Provider).Equals(app.FacebookProvider)
return app.ErrNotFound
})

bus.AddHandler(func(ctx context.Context, q *query.GetOAuthProfile) error {
if q.Provider == app.FacebookProvider && q.Code == "456" {
q.Result = &dto.OAuthUserProfile{
ID: "FB456",
Name: "Some Facebook Guy",
Email: "some.guy@facebook.com",
}
return nil
Expect(q.Provider).Equals(app.FacebookProvider)
Expect(q.Code).Equals("456")
q.Result = &dto.OAuthUserProfile{
ID: "FB456",
Name: "Some Facebook Guy",
Email: "some.guy@facebook.com",
}
return app.ErrNotFound
return nil
})

code, response := server.
Expand All @@ -453,6 +470,73 @@ func TestOAuthTokenHandler_NewUser_PrivateTenant(t *testing.T) {
ExpectFiderAuthCookie(response, nil)
}

func TestOAuthTokenHandler_NewUser_PrivateSite_UsingTrustedProvider(t *testing.T) {
RegisterT(t)

server := mock.NewServer()
mock.AvengersTenant.IsPrivate = true

providerCode := "_jd72hfjv"

bus.AddHandler(func(ctx context.Context, c *cmd.RegisterUser) error {
Expect(c.User.Name).Equals("Mark Doe")
Expect(c.User.Email).Equals("mark.doe@microsoft.com")
Expect(c.User.Providers).HasLen(1)
Expect(c.User.Providers[0].UID).Equals("1234-5678")
Expect(c.User.Providers[0].Name).Equals(providerCode)
Expect(c.User.Role).Equals(enum.RoleVisitor)

c.User.ID = 999
return nil
})

bus.AddHandler(func(ctx context.Context, q *query.GetUserByEmail) error {
return app.ErrNotFound
})

bus.AddHandler(func(ctx context.Context, q *query.GetUserByProvider) error {
return app.ErrNotFound
})

bus.AddHandler(func(ctx context.Context, q *query.GetCustomOAuthConfigByProvider) error {
Expect(q.Provider).Equals(providerCode)
q.Result = &entity.OAuthConfig{
Provider: providerCode,
DisplayName: "Microsoft AD",
IsTrusted: true,
}
return nil
})

bus.AddHandler(func(ctx context.Context, q *query.GetOAuthProfile) error {
Expect(q.Provider).Equals(providerCode)
Expect(q.Code).Equals("000111")
q.Result = &dto.OAuthUserProfile{
ID: "1234-5678",
Name: "Mark Doe",
Email: "mark.doe@microsoft.com",
}
return nil
})

code, response := server.
WithURL("http://feedback.theavengers.com/oauth/"+providerCode+"/token?code=000111&identifier=MY_SESSION_ID&redirect=/").
OnTenant(mock.AvengersTenant).
AddParam("provider", providerCode).
AddCookie(web.CookieSessionName, "MY_SESSION_ID").
Use(middlewares.Session()).
Execute(handlers.OAuthToken())

Expect(code).Equals(http.StatusTemporaryRedirect)

Expect(response.Header().Get("Location")).Equals("/")
ExpectFiderAuthCookie(response, &entity.User{
ID: 999,
Name: "Mark Doe",
Email: "mark.doe@microsoft.com",
})
}

func TestOAuthTokenHandler_InvalidIdentifier(t *testing.T) {
RegisterT(t)
server := mock.NewServer()
Expand Down
1 change: 1 addition & 0 deletions app/models/cmd/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type SaveCustomOAuthConfig struct {
TokenURL string
Scope string
ProfileURL string
IsTrusted bool
JSONUserIDPath string
JSONUserNamePath string
JSONUserEmailPath string
Expand Down
2 changes: 2 additions & 0 deletions app/models/entity/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type OAuthConfig struct {
TokenURL string
ProfileURL string
Scope string
IsTrusted bool
JSONUserIDPath string
JSONUserNamePath string
JSONUserEmailPath string
Expand All @@ -38,6 +39,7 @@ func (o OAuthConfig) MarshalJSON() ([]byte, error) {
"tokenURL": o.TokenURL,
"profileURL": o.ProfileURL,
"scope": o.Scope,
"isTrusted": o.IsTrusted,
"jsonUserIDPath": o.JSONUserIDPath,
"jsonUserNamePath": o.JSONUserNamePath,
"jsonUserEmailPath": o.JSONUserEmailPath,
Expand Down
3 changes: 3 additions & 0 deletions app/services/oauth/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ var (
DisplayName: "Facebook",
ProfileURL: "https://graph.facebook.com/me?fields=name,email",
Status: getProviderStatus(env.Config.OAuth.Facebook.AppID),
IsTrusted: false,
ClientID: env.Config.OAuth.Facebook.AppID,
ClientSecret: env.Config.OAuth.Facebook.Secret,
Scope: "public_profile email",
Expand All @@ -79,6 +80,7 @@ var (
DisplayName: "Google",
ProfileURL: "https://www.googleapis.com/oauth2/v2/userinfo",
Status: getProviderStatus(env.Config.OAuth.Google.ClientID),
IsTrusted: false,
ClientID: env.Config.OAuth.Google.ClientID,
ClientSecret: env.Config.OAuth.Google.Secret,
Scope: "profile email",
Expand All @@ -93,6 +95,7 @@ var (
DisplayName: "GitHub",
ProfileURL: "https://api.github.com/user",
Status: getProviderStatus(env.Config.OAuth.GitHub.ClientID),
IsTrusted: false,
ClientID: env.Config.OAuth.GitHub.ClientID,
ClientSecret: env.Config.OAuth.GitHub.Secret,
Scope: "user:email",
Expand Down
16 changes: 9 additions & 7 deletions app/services/sqlstore/postgres/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type dbOAuthConfig struct {
DisplayName string `db:"display_name"`
LogoBlobKey string `db:"logo_bkey"`
Status int `db:"status"`
IsTrusted bool `db:"is_trusted"`
ClientID string `db:"client_id"`
ClientSecret string `db:"client_secret"`
AuthorizeURL string `db:"authorize_url"`
Expand All @@ -35,6 +36,7 @@ func (m *dbOAuthConfig) toModel() *entity.OAuthConfig {
Provider: m.Provider,
DisplayName: m.DisplayName,
Status: m.Status,
IsTrusted: m.IsTrusted,
LogoBlobKey: m.LogoBlobKey,
ClientID: m.ClientID,
ClientSecret: m.ClientSecret,
Expand All @@ -56,7 +58,7 @@ func getCustomOAuthConfigByProvider(ctx context.Context, q *query.GetCustomOAuth

config := &dbOAuthConfig{}
err := trx.Get(config, `
SELECT id, provider, display_name, status, logo_bkey,
SELECT id, provider, display_name, status, is_trusted, logo_bkey,
client_id, client_secret, authorize_url,
profile_url, token_url, scope, json_user_id_path,
json_user_name_path, json_user_email_path
Expand All @@ -81,7 +83,7 @@ func listCustomOAuthConfig(ctx context.Context, q *query.ListCustomOAuthConfig)
configs := []*dbOAuthConfig{}
if tenant != nil {
err := trx.Select(&configs, `
SELECT id, provider, display_name, status, logo_bkey,
SELECT id, provider, display_name, status, is_trusted, logo_bkey,
client_id, client_secret, authorize_url,
profile_url, token_url, scope, json_user_id_path,
json_user_name_path, json_user_email_path
Expand Down Expand Up @@ -111,15 +113,15 @@ func saveCustomOAuthConfig(ctx context.Context, c *cmd.SaveCustomOAuthConfig) er

if c.ID == 0 {
query := `INSERT INTO oauth_providers (
tenant_id, provider, display_name, status,
tenant_id, provider, display_name, status, is_trusted,
client_id, client_secret, authorize_url,
profile_url, token_url, scope, json_user_id_path,
json_user_name_path, json_user_email_path, logo_bkey
) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14)
) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15)
RETURNING id`

err = trx.Get(&c.ID, query, tenant.ID, c.Provider,
c.DisplayName, c.Status, c.ClientID, c.ClientSecret,
c.DisplayName, c.Status, c.IsTrusted, c.ClientID, c.ClientSecret,
c.AuthorizeURL, c.ProfileURL, c.TokenURL,
c.Scope, c.JSONUserIDPath, c.JSONUserNamePath,
c.JSONUserEmailPath, c.Logo.BlobKey)
Expand All @@ -129,14 +131,14 @@ func saveCustomOAuthConfig(ctx context.Context, c *cmd.SaveCustomOAuthConfig) er
SET display_name = $3, status = $4, client_id = $5, client_secret = $6,
authorize_url = $7, profile_url = $8, token_url = $9, scope = $10,
json_user_id_path = $11, json_user_name_path = $12, json_user_email_path = $13,
logo_bkey = $14
logo_bkey = $14, is_trusted = $15
WHERE tenant_id = $1 AND id = $2`

_, err = trx.Execute(query, tenant.ID, c.ID,
c.DisplayName, c.Status, c.ClientID, c.ClientSecret,
c.AuthorizeURL, c.ProfileURL, c.TokenURL,
c.Scope, c.JSONUserIDPath, c.JSONUserNamePath,
c.JSONUserEmailPath, c.Logo.BlobKey)
c.JSONUserEmailPath, c.Logo.BlobKey, c.IsTrusted)
}

if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions migrations/202205082055_trusted_provider.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE oauth_providers ADD is_trusted BOOLEAN default false;

CREATE UNIQUE INDEX oauth_provider_uq ON oauth_providers (provider);