From 14127b2180fba4f33b30449a9b1704d4deda632f Mon Sep 17 00:00:00 2001 From: Chris Penner <christopher.penner@gmail.com> Date: Fri, 11 Apr 2025 10:15:33 -0700 Subject: [PATCH 1/7] Spec out alternative local github flow --- src/Share/Github.hs | 30 +++++++------- src/Share/Postgres/Users/Queries.hs | 2 +- src/Share/Web/OAuth/Impl.hs | 63 ++++++++++++++++++++++------- 3 files changed, 64 insertions(+), 31 deletions(-) diff --git a/src/Share/Github.hs b/src/Share/Github.hs index 8c962d64..c41e748f 100644 --- a/src/Share/Github.hs +++ b/src/Share/Github.hs @@ -45,10 +45,10 @@ instance ToServerError GithubError where GithubDecodeError _ce -> (ErrorID "github:decode-error", err500 {errBody = "Github returned an unexpected response. Please try again."}) data GithubUser = GithubUser - { github_user_login :: Text, - github_user_id :: Int64, - github_user_avatar_url :: URIParam, - github_user_name :: Maybe Text + { githubHandle :: Text, + githubUserId :: Int64, + githubUserAvatarUrl :: URIParam, + githubUserName :: Maybe Text } deriving (Show) @@ -56,21 +56,21 @@ instance FromJSON GithubUser where parseJSON = withObject "GithubUser" $ \u -> GithubUser <$> u - .: "login" + .: "login" <*> u - .: "id" + .: "id" <*> u - .: "avatar_url" + .: "avatar_url" -- We don't use this email because it's the "publicly visible" email, instead we fetch -- the primary email using the emails API. -- <*> u .: "email" <*> u - .:? "name" + .:? "name" data GithubEmail = GithubEmail - { github_email_email :: Text, - github_email_primary :: Bool, - github_email_verified :: Bool + { githubEmailEmail :: Text, + githubEmailIsPrimary :: Bool, + githubEmailIsVerified :: Bool } deriving (Show) @@ -78,11 +78,11 @@ instance FromJSON GithubEmail where parseJSON = withObject "GithubEmail" $ \u -> GithubEmail <$> u - .: "email" + .: "email" <*> u - .: "primary" + .: "primary" <*> u - .: "verified" + .: "verified" type GithubTokenApi = "login" @@ -190,7 +190,7 @@ primaryGithubEmail auth = do emails <- runGithubClient githubAPIBaseURL (githubEmailsClient auth) -- Github's api docs suggest there will always be a primary email. -- https://docs.github.com/en/rest/users/emails#list-email-addresses-for-the-authenticated-user - case find github_email_primary emails of + case find githubEmailIsPrimary emails of Nothing -> respondError GithubUserWithoutPrimaryEmail Just email -> pure email diff --git a/src/Share/Postgres/Users/Queries.hs b/src/Share/Postgres/Users/Queries.hs index 4b50bc0a..153902dc 100644 --- a/src/Share/Postgres/Users/Queries.hs +++ b/src/Share/Postgres/Users/Queries.hs @@ -197,7 +197,7 @@ userByHandle handle = do createFromGithubUser :: AuthZ.AuthZReceipt -> GithubUser -> GithubEmail -> PG.Transaction UserCreationError User createFromGithubUser !authzReceipt (GithubUser githubHandle githubUserId avatar_url user_name) primaryEmail = do - let (GithubEmail {github_email_email = user_email, github_email_verified = emailVerified}) = primaryEmail + let (GithubEmail {githubEmailEmail = user_email, githubEmailIsVerified = emailVerified}) = primaryEmail userHandle <- case IDs.fromText @UserHandle (Text.toLower githubHandle) of Left err -> throwError (InvalidUserHandle err githubHandle) Right handle -> pure handle diff --git a/src/Share/Web/OAuth/Impl.hs b/src/Share/Web/OAuth/Impl.hs index 401315c2..90985b28 100644 --- a/src/Share/Web/OAuth/Impl.hs +++ b/src/Share/Web/OAuth/Impl.hs @@ -17,7 +17,9 @@ where import Control.Monad import Control.Monad.Reader import Data.Map qualified as Map +import Data.Maybe (fromJust) import Data.Set qualified as Set +import Network.URI (parseURI) import Servant import Share.App (shareAud, shareIssuer) import Share.Env qualified as Env @@ -35,11 +37,11 @@ import Share.OAuth.Session qualified as Session import Share.OAuth.Types (AccessToken, AuthenticationRequest (..), Code, GrantType (AuthorizationCode), OAuth2State, OAuthClientConfig (..), OAuthClientId, PKCEChallenge, PKCEChallengeMethod, RedirectReceiverErr (..), ResponseType (ResponseTypeCode), TokenRequest (..), TokenResponse (..), TokenType (BearerToken)) import Share.OAuth.Types qualified as OAuth import Share.Postgres qualified as PG -import Share.Postgres.Ops qualified as PGO import Share.Postgres.Users.Queries qualified as UserQ import Share.Prelude import Share.User (User (User)) import Share.User qualified as User +import Share.Utils.Deployment qualified as Deployment import Share.Utils.Logging qualified as Logging import Share.Utils.Servant import Share.Utils.Servant.Cookies (CookieVal, cookieVal) @@ -136,22 +138,24 @@ redirectReceiverEndpoint _mayGithubCode _mayStatePSID (Just errorType) mayErrorD otherErrType -> do Logging.logErrorText ("Github authentication error: " <> otherErrType <> " " <> fold mayErrorDescription) errorRedirect UnspecifiedError -redirectReceiverEndpoint mayGithubCode mayStatePSID _errorType@Nothing _mayErrorDescription mayCookiePSID existingAuthSession = do +redirectReceiverEndpoint mayGithubCode mayStatePSID _errorType@Nothing _mayErrorDescription mayCookiePSID _existingAuthSession = do cookiePSID <- case cookieVal mayCookiePSID of Nothing -> respondError $ MissingOrExpiredPendingSession Just psid -> pure psid PendingSession {loginRequest, returnToURI = unvalidatedReturnToURI} <- ensurePendingSession cookiePSID - newOrPreExistingUser <- case (mayGithubCode, mayStatePSID, existingAuthSession) of - -- The user has an already valid session, we can use that. - (_, _, Just session) -> do - user <- (PGO.expectUserById (sessionUserId session)) - pure (UserQ.PreExisting user) + newOrPreExistingUser <- case (mayGithubCode, mayStatePSID) of -- The user has completed the Github flow, we can log them in or create a new user. - (Just githubCode, Just statePSID, _noSession) -> do - completeGithubFlow githubCode statePSID cookiePSID - (Nothing, _, _) -> do + (Just githubCode, Just statePSID) -> do + (ghUser, ghEmail) <- + -- Skip the github flow when developing locally, and just use some dummy github user + -- data. + if Deployment.onLocal + then pure localGithubUserInfo + else getGithubUserInfo githubCode statePSID cookiePSID + completeGithubFlow ghUser ghEmail + (Nothing, _) -> do respondError $ MissingCode - (_, Nothing, _) -> do + (_, Nothing) -> do respondError $ MissingState let (User {User.user_id = uid}) = UserQ.getNewOrPreExisting newOrPreExistingUser when (UserQ.isNew newOrPreExistingUser) do @@ -186,19 +190,37 @@ redirectReceiverEndpoint mayGithubCode mayStatePSID _errorType@Nothing _mayError Nothing -> respondError $ InternalServerError "session-create-failure" ("Failed to create session" :: Text) Just setAuthHeaders -> pure . clearPendingSessionCookie cookieSettings $ setAuthHeaders response where - completeGithubFlow :: + localGithubUserInfo :: (Github.GithubUser, Github.GithubEmail) + localGithubUserInfo = + ( Github.GithubUser + { githubHandle = "LocalGithubUser", + githubUserId = 1, + githubUserAvatarUrl = URIParam $ fromJust $ parseURI "https://avatars.githubusercontent.com/u/0?v=4", + githubUserName = Just "Local Github User" + }, + Github.GithubEmail + { githubEmailEmail = "local@example.com", + githubEmailIsPrimary = True, + githubEmailIsVerified = True + } + ) + + getGithubUserInfo :: ( OAuth.Code -> PendingSessionId -> PendingSessionId -> - WebApp (UserQ.NewOrPreExisting User) + WebApp (Github.GithubUser, Github.GithubEmail) ) - completeGithubFlow githubCode statePSID cookiePSID = do + getGithubUserInfo githubCode statePSID cookiePSID = do when (statePSID /= cookiePSID) do Redis.liftRedis $ Redis.failPendingSession cookiePSID respondError (MismatchedState cookiePSID statePSID) token <- Github.githubTokenForCode githubCode ghUser <- Github.githubUser token ghEmail <- Github.primaryGithubEmail token + pure (ghUser, ghEmail) + completeGithubFlow :: Github.GithubUser -> Github.GithubEmail -> WebApp (UserQ.NewOrPreExisting User) + completeGithubFlow ghUser ghEmail = do PG.tryRunTransaction (UserQ.findOrCreateGithubUser AuthZ.userCreationOverride ghUser ghEmail) >>= \case Left (UserQ.UserHandleTaken _) -> do errorRedirect AccountCreationHandleAlreadyTaken @@ -256,8 +278,19 @@ loginWithGithub :: NoContent ) loginWithGithub psid = do - githubAuthURI <- Github.githubAuthenticationURI psid + githubAuthURI <- + if Deployment.onLocal + then skipGithubLoginURL psid + else Github.githubAuthenticationURI psid pure $ redirectTo githubAuthURI + where + skipGithubLoginURL :: OAuth2State -> WebApp URI + skipGithubLoginURL oauth2State = do + sharePathQ ["oauth", "redirect"] $ + Map.fromList + [ ("code", "code"), + ("state", toQueryParam oauth2State) + ] -- | Log out the user by telling the browser to clear the session cookies. -- Note that this doesn't (yet) invalidate the session itself, it just removes its cookie from the From fa595a1da2204fd90ee101803c7adccfe00dc466 Mon Sep 17 00:00:00 2001 From: Chris Penner <christopher.penner@gmail.com> Date: Fri, 11 Apr 2025 10:41:39 -0700 Subject: [PATCH 2/7] Add local user-creation transcripts --- transcripts/run-transcripts.zsh | 1 + .../share-apis/user-creation/new-user-login.json | 1 + .../user-creation/new-user-profile.json | 16 ++++++++++++++++ transcripts/share-apis/user-creation/run.zsh | 14 ++++++++++++++ 4 files changed, 32 insertions(+) create mode 100644 transcripts/share-apis/user-creation/new-user-login.json create mode 100644 transcripts/share-apis/user-creation/new-user-profile.json create mode 100755 transcripts/share-apis/user-creation/run.zsh diff --git a/transcripts/run-transcripts.zsh b/transcripts/run-transcripts.zsh index 87b58787..a8615ff6 100755 --- a/transcripts/run-transcripts.zsh +++ b/transcripts/run-transcripts.zsh @@ -16,6 +16,7 @@ transcripts=( contribution-merge transcripts/share-apis/contribution-merge/ search transcripts/share-apis/search/ users transcripts/share-apis/users/ + user-creation transcripts/share-apis/user-creation/ contribution-diffs transcripts/share-apis/contribution-diffs/ definition-diffs transcripts/share-apis/definition-diffs/ tickets transcripts/share-apis/tickets/ diff --git a/transcripts/share-apis/user-creation/new-user-login.json b/transcripts/share-apis/user-creation/new-user-login.json new file mode 100644 index 00000000..ab43a272 --- /dev/null +++ b/transcripts/share-apis/user-creation/new-user-login.json @@ -0,0 +1 @@ +{"status_code": 200, "result_url": "http://localhost:1234/?event=new-user-log-in"} \ No newline at end of file diff --git a/transcripts/share-apis/user-creation/new-user-profile.json b/transcripts/share-apis/user-creation/new-user-profile.json new file mode 100644 index 00000000..b6db0553 --- /dev/null +++ b/transcripts/share-apis/user-creation/new-user-profile.json @@ -0,0 +1,16 @@ +{ + "body": { + "avatarUrl": "https://avatars.githubusercontent.com/u/0?v=4", + "completedTours": [], + "handle": "localgithubuser", + "name": "Local Github User", + "organizationMemberships": [], + "primaryEmail": "local@example.com", + "userId": "U-<UUID>" + }, + "status": [ + { + "status_code": 200 + } + ] +} diff --git a/transcripts/share-apis/user-creation/run.zsh b/transcripts/share-apis/user-creation/run.zsh new file mode 100755 index 00000000..cb9aa427 --- /dev/null +++ b/transcripts/share-apis/user-creation/run.zsh @@ -0,0 +1,14 @@ +#!/usr/bin/env zsh + +set -e + +source "../../transcript_helpers.sh" + +# Create a cookie jar we can use to store cookies for the new user we're goin to create. +new_user_cookie_jar=$(cookie_jar_for_user_id "new_user") + +# Should be able to create a new user via the login flow by following redirects. +curl -s -L -I -o /dev/null -w '{"status_code": %{http_code}, "result_url": "%{url_effective}"}' --request "GET" --cookie "$new_user_cookie_jar" --cookie-jar "$new_user_cookie_jar" "localhost:5424/login" > ./new-user-login.json + +# user should now be logged in as the local github user. +fetch "new_user" GET new-user-profile /account From 873f00b9e132a4670fd0d5973eee261eb0f6d7bd Mon Sep 17 00:00:00 2001 From: Chris Penner <christopher.penner@gmail.com> Date: Fri, 11 Apr 2025 15:59:38 -0700 Subject: [PATCH 3/7] Testing CI failures --- transcripts/share-apis/user-creation/run.zsh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/transcripts/share-apis/user-creation/run.zsh b/transcripts/share-apis/user-creation/run.zsh index cb9aa427..a8978a29 100755 --- a/transcripts/share-apis/user-creation/run.zsh +++ b/transcripts/share-apis/user-creation/run.zsh @@ -1,6 +1,6 @@ #!/usr/bin/env zsh -set -e +set -ex source "../../transcript_helpers.sh" @@ -8,7 +8,12 @@ source "../../transcript_helpers.sh" new_user_cookie_jar=$(cookie_jar_for_user_id "new_user") # Should be able to create a new user via the login flow by following redirects. -curl -s -L -I -o /dev/null -w '{"status_code": %{http_code}, "result_url": "%{url_effective}"}' --request "GET" --cookie "$new_user_cookie_jar" --cookie-jar "$new_user_cookie_jar" "localhost:5424/login" > ./new-user-login.json +curl -v -s -L -I -o /dev/null -w '{"status_code": %{http_code}, "result_url": "%{url_effective}"}' --request "GET" --cookie "$new_user_cookie_jar" --cookie-jar "$new_user_cookie_jar" "localhost:5424/login" > ./new-user-login.json + +echo "Created new user!" # user should now be logged in as the local github user. fetch "new_user" GET new-user-profile /account + + +echo "DONE" From 59d463cf823a1cbed3b5c0578b47ae89c2913bb9 Mon Sep 17 00:00:00 2001 From: Chris Penner <christopher.penner@gmail.com> Date: Mon, 28 Apr 2025 10:36:52 -0700 Subject: [PATCH 4/7] Clean up new user tests --- .../share-apis/user-creation/new-user-login.json | 2 +- .../share-apis/user-creation/new-user-profile.json | 1 + transcripts/share-apis/user-creation/run.zsh | 11 ++++------- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/transcripts/share-apis/user-creation/new-user-login.json b/transcripts/share-apis/user-creation/new-user-login.json index ab43a272..fbf2553e 100644 --- a/transcripts/share-apis/user-creation/new-user-login.json +++ b/transcripts/share-apis/user-creation/new-user-login.json @@ -1 +1 @@ -{"status_code": 200, "result_url": "http://localhost:1234/?event=new-user-log-in"} \ No newline at end of file +{"result_url": "http://localhost:1234/?event=new-user-log-in"} \ No newline at end of file diff --git a/transcripts/share-apis/user-creation/new-user-profile.json b/transcripts/share-apis/user-creation/new-user-profile.json index b6db0553..79139537 100644 --- a/transcripts/share-apis/user-creation/new-user-profile.json +++ b/transcripts/share-apis/user-creation/new-user-profile.json @@ -3,6 +3,7 @@ "avatarUrl": "https://avatars.githubusercontent.com/u/0?v=4", "completedTours": [], "handle": "localgithubuser", + "isSuperadmin": false, "name": "Local Github User", "organizationMemberships": [], "primaryEmail": "local@example.com", diff --git a/transcripts/share-apis/user-creation/run.zsh b/transcripts/share-apis/user-creation/run.zsh index a8978a29..6857cbef 100755 --- a/transcripts/share-apis/user-creation/run.zsh +++ b/transcripts/share-apis/user-creation/run.zsh @@ -1,6 +1,6 @@ #!/usr/bin/env zsh -set -ex +set -e source "../../transcript_helpers.sh" @@ -8,12 +8,9 @@ source "../../transcript_helpers.sh" new_user_cookie_jar=$(cookie_jar_for_user_id "new_user") # Should be able to create a new user via the login flow by following redirects. -curl -v -s -L -I -o /dev/null -w '{"status_code": %{http_code}, "result_url": "%{url_effective}"}' --request "GET" --cookie "$new_user_cookie_jar" --cookie-jar "$new_user_cookie_jar" "localhost:5424/login" > ./new-user-login.json - -echo "Created new user!" +# Note that the end of the redirect chain ends up on the Share UI (localhost:1234) which may or may not be running, so +# we just ignore bad status codes from that server. +curl -s -L -I -o /dev/null -w '{"result_url": "%{url_effective}"}' --request "GET" --cookie "$new_user_cookie_jar" --cookie-jar "$new_user_cookie_jar" "localhost:5424/login" > ./new-user-login.json || true # user should now be logged in as the local github user. fetch "new_user" GET new-user-profile /account - - -echo "DONE" From 577eb0d6aa42005efb270fa8f30f7d163c91e535 Mon Sep 17 00:00:00 2001 From: Chris Penner <christopher.penner@gmail.com> Date: Mon, 28 Apr 2025 10:50:48 -0700 Subject: [PATCH 5/7] Set up for docker in CI transcripts --- docker/docker-compose.yml | 4 ++-- transcripts/share-apis/user-creation/new-user-login.json | 1 - transcripts/share-apis/user-creation/run.zsh | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) delete mode 100644 transcripts/share-apis/user-creation/new-user-login.json diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 62f743a2..e46527d7 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -48,13 +48,13 @@ services: environment: # Placeholder values for development - - SHARE_API_ORIGIN=http://share-api + - SHARE_DEPLOYMENT=local + - SHARE_API_ORIGIN=http://share:5424 - SHARE_SERVER_PORT=5424 - SHARE_REDIS=redis://redis:6379 - SHARE_POSTGRES=postgresql://postgres:sekrit@postgres:5432 - SHARE_HMAC_KEY=hmac-key-test-key-test-key-test- - SHARE_EDDSA_KEY=eddsa-key-test-key-test-key-test - - SHARE_DEPLOYMENT=local - SHARE_POSTGRES_CONN_TTL=30 - SHARE_POSTGRES_CONN_MAX=10 - SHARE_SHARE_UI_ORIGIN=http://localhost:1234 diff --git a/transcripts/share-apis/user-creation/new-user-login.json b/transcripts/share-apis/user-creation/new-user-login.json deleted file mode 100644 index fbf2553e..00000000 --- a/transcripts/share-apis/user-creation/new-user-login.json +++ /dev/null @@ -1 +0,0 @@ -{"result_url": "http://localhost:1234/?event=new-user-log-in"} \ No newline at end of file diff --git a/transcripts/share-apis/user-creation/run.zsh b/transcripts/share-apis/user-creation/run.zsh index 6857cbef..88aa1eee 100755 --- a/transcripts/share-apis/user-creation/run.zsh +++ b/transcripts/share-apis/user-creation/run.zsh @@ -10,7 +10,7 @@ new_user_cookie_jar=$(cookie_jar_for_user_id "new_user") # Should be able to create a new user via the login flow by following redirects. # Note that the end of the redirect chain ends up on the Share UI (localhost:1234) which may or may not be running, so # we just ignore bad status codes from that server. -curl -s -L -I -o /dev/null -w '{"result_url": "%{url_effective}"}' --request "GET" --cookie "$new_user_cookie_jar" --cookie-jar "$new_user_cookie_jar" "localhost:5424/login" > ./new-user-login.json || true +curl -s -L -I -o /dev/null -w '{"result_url": "%{url_effective}"}' --request "GET" --cookie "$new_user_cookie_jar" --cookie-jar "$new_user_cookie_jar" "localhost:5424/login" || true # user should now be logged in as the local github user. fetch "new_user" GET new-user-profile /account From da74ef7b0c7944e5bd1e5f7c5b74ca900f1f2095 Mon Sep 17 00:00:00 2001 From: Chris Penner <christopher.penner@gmail.com> Date: Mon, 28 Apr 2025 11:14:56 -0700 Subject: [PATCH 6/7] Debugging CI transcripts --- transcripts/share-apis/user-creation/run.zsh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transcripts/share-apis/user-creation/run.zsh b/transcripts/share-apis/user-creation/run.zsh index 88aa1eee..895a4afd 100755 --- a/transcripts/share-apis/user-creation/run.zsh +++ b/transcripts/share-apis/user-creation/run.zsh @@ -10,7 +10,7 @@ new_user_cookie_jar=$(cookie_jar_for_user_id "new_user") # Should be able to create a new user via the login flow by following redirects. # Note that the end of the redirect chain ends up on the Share UI (localhost:1234) which may or may not be running, so # we just ignore bad status codes from that server. -curl -s -L -I -o /dev/null -w '{"result_url": "%{url_effective}"}' --request "GET" --cookie "$new_user_cookie_jar" --cookie-jar "$new_user_cookie_jar" "localhost:5424/login" || true +curl -v -L -I -o /dev/null -w '{"result_url": "%{url_effective}"}' --request "GET" --cookie "$new_user_cookie_jar" --cookie-jar "$new_user_cookie_jar" "http://localhost:5424/login" || true # user should now be logged in as the local github user. fetch "new_user" GET new-user-profile /account From f2a21bfa51e382912d7f3df7dc602eb5b4eaee95 Mon Sep 17 00:00:00 2001 From: Chris Penner <christopher.penner@gmail.com> Date: Mon, 28 Apr 2025 12:58:12 -0700 Subject: [PATCH 7/7] Try localhost as share api --- docker/docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index e46527d7..0efd5c19 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -49,7 +49,7 @@ services: environment: # Placeholder values for development - SHARE_DEPLOYMENT=local - - SHARE_API_ORIGIN=http://share:5424 + - SHARE_API_ORIGIN=http://localhost:5424 - SHARE_SERVER_PORT=5424 - SHARE_REDIS=redis://redis:6379 - SHARE_POSTGRES=postgresql://postgres:sekrit@postgres:5432