Skip to content

Commit

Permalink
Merge pull request #240 from camphor-/refactor-reliable-origins
Browse files Browse the repository at this point in the history
  • Loading branch information
p1ass authored Jan 8, 2022
2 parents ec23563 + 5ec4c09 commit c62d4cc
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 62 deletions.
25 changes: 25 additions & 0 deletions config/origin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package config

import "regexp"

// IsReliableOrigin は受け取ったURLが信頼できるURLか確認します。
func IsReliableOrigin(url string) bool {
if IsDev() {
return isReliableOriginDev(url)
}
// ローカルは何でも良い
if IsLocal() {
return true
}

return isReliableOrigin(url)
}

func isReliableOrigin(url string) bool {
return FrontendURL() == url
}

func isReliableOriginDev(url string) bool {
re := regexp.MustCompile(`^https://[a-zA-Z0-9-_]+\.relaym\.pages\.dev$`)
return re.MatchString(url)
}
74 changes: 74 additions & 0 deletions config/origin_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package config

import (
"os"
"testing"
)

func Test_origin_IsReliableOrigin(t *testing.T) {
t.Parallel()

tests := []struct {
name string
origin string
env string
want bool
}{
{
name: "Cloudflare Pages: 正しいデプロイプレビューのURLならtrue",
origin: "https://5c927597.relaym.pages.dev",
env: "dev",
want: true,
},
{
name: "Cloudflare Pages: 別のURLならfalse",
origin: "https://5c927597.relaym2.pages.dev",
env: "dev",
want: false,
},
{
name: "Cloudflare Pages: 前後に変な文字が入ってもfalse",
origin: "https://evil.example.com/https://5c927597.relaym2.pages.dev/hoge",
env: "dev",
want: false,
},
{
name: "Cloudflare Pages: クエリパラメータで回避しようとしてもfalse",
origin: "https://evil.com?hoge=5c927597.relaym.pages.dev",
env: "dev",
want: false,
},
{
name: "ローカル環境は何でも良い",
origin: "http://relaym.local:3000",
env: "local",
want: true,
},
{
name: "本番環境では、FRONTEND_URLと同じ値のときはtrueになる",
origin: "http://relaym.local:3000", // test時にセットされるFRONTEND_URLの環境変数がこれになっている。本番環境では本番のURLが環境変数で渡される。
env: "prod",
want: true,
},
{
name: "本番環境では、FRONTEND_URLと違う値のときはfalseになる",
origin: "http://relaym.local:3001",
env: "prod",
want: false,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
originalEnv := os.Getenv("ENV")
os.Setenv("ENV", tt.env)
defer func() {
os.Setenv("ENV", originalEnv)
}()
if got := IsReliableOrigin(tt.origin); got != tt.want {
t.Errorf("IsReliableOrigin() = %v, want %v", got, tt.want)
}
})
}
}
2 changes: 1 addition & 1 deletion docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ JavaScriptで非同期にリクエストするのではなく、aタグで同期

| key | 説明 |
| --- | ------- |
| redirect_url | Spotifyの認証が終わった後リダイレクトされるクライアントのURLを指定します |
| redirect_url | Spotifyの認証が終わった後リダイレクトされるクライアントのURLを指定します。主に、開発環境やローカル環境のためのクエリパラメータで、本番環境では環境変数にセットされた値が用いられます。 |

#### レスポンス
| code | 補足 |
Expand Down
9 changes: 5 additions & 4 deletions usecase/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func (u *AuthUseCase) GetAuthURL(redirectURL string) (string, error) {

// Authorization はcodeを使って認可をチェックします。
// 認可に成功した場合はフロントエンドのリダイレクトURLとセッションIDを返します。
// リダイレクトURLは空である可能性がある点に注意してください。
func (u *AuthUseCase) Authorization(state, code string) (string, string, error) {
storedState, err := u.repo.FindStateByState(state)
if err != nil {
Expand All @@ -53,22 +54,22 @@ func (u *AuthUseCase) Authorization(state, code string) (string, string, error)

token, err := u.authCli.Exchange(code)
if err != nil {
return "", "", fmt.Errorf("exchange and get oauth2 token: %w", err)
return storedState.RedirectURL, "", fmt.Errorf("exchange and get oauth2 token: %w", err)
}

ctx := service.SetTokenToContext(context.Background(), token)
userID, err := u.createUserIfNotExists(ctx)
if err != nil {
return "", "", fmt.Errorf("get or create user: %w", err)
return storedState.RedirectURL, "", fmt.Errorf("get or create user: %w", err)
}

if err := u.repo.StoreORUpdateToken(userID, token); err != nil {
return "", "", fmt.Errorf("store or update oauth token though repo userID=%s: %w", userID, err)
return storedState.RedirectURL, "", fmt.Errorf("store or update oauth token though repo userID=%s: %w", userID, err)
}

sessionID := uuid.New().String()
if err := u.repo.StoreSession(sessionID, userID); err != nil {
return "", "", fmt.Errorf("store session sessionID=%s userID=%s : %w", sessionID, userID, err)
return storedState.RedirectURL, "", fmt.Errorf("store session sessionID=%s userID=%s : %w", sessionID, userID, err)
}

// Stateを削除するのが失敗してもログインは成功しているので、エラーを返さない
Expand Down
54 changes: 0 additions & 54 deletions web/deploy_preview_cors_middleware_test.go

This file was deleted.

9 changes: 6 additions & 3 deletions web/handler/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ func (h *AuthHandler) Login(c echo.Context) error {
logger := log.New()

redirectURL := c.QueryParam("redirect_url")
if redirectURL == "" {
if redirectURL == "" || !config.IsReliableOrigin(redirectURL) {
redirectURL = h.frontendURL
}
url, err := h.authUC.GetAuthURL(redirectURL)
if err != nil {
logger.Errorj(map[string]interface{}{"message": "failed to get auth url", "error": err.Error()})
return c.Redirect(http.StatusFound, h.frontendURL+"?err=spotifyAuthFailed")
return c.Redirect(http.StatusFound, redirectURL+"?err=spotifyAuthFailed")
}

return c.Redirect(http.StatusFound, url)
Expand Down Expand Up @@ -62,7 +62,10 @@ func (h *AuthHandler) Callback(c echo.Context) error {
redirectURL, sessionID, err := h.authUC.Authorization(state, code)
if err != nil {
logger.Errorj(map[string]interface{}{"message": "spotify auth failed", "error": err.Error()})
return c.Redirect(http.StatusFound, h.frontendURL+"?err=spotifyAuthFailed")
if redirectURL == "" {
return c.Redirect(http.StatusFound, h.frontendURL+"?err=spotifyAuthFailed")
}
return c.Redirect(http.StatusFound, redirectURL+"?err=spotifyAuthFailed")
}

sameSite := http.SameSiteNoneMode
Expand Down

0 comments on commit c62d4cc

Please sign in to comment.