From 182fcf643fb5230ee852f3ed5c5091cbee239787 Mon Sep 17 00:00:00 2001 From: wancat Date: Fri, 13 Oct 2023 00:05:59 +0800 Subject: [PATCH 1/8] feat: pick username * put new registered social login user to session data * ask to choose username * simplify checkUsername functions --- app/src/components/AuthenticationPart.vue | 24 +++--- app/src/components/UsernameForm.vue | 82 ++++++++++++++++++ app/src/router.js | 7 ++ app/src/views/ChooseUsernamePage.vue | 16 ++++ server/src/api/controllers/authController.js | 89 +++++++++++++++----- server/src/api/routes/auth.js | 20 ++--- server/src/auth/githubStrategy.js | 14 +-- server/src/auth/googleStrategy.js | 10 ++- server/src/middleware/authRequired.js | 2 +- server/src/models/user.js | 3 +- server/src/utils/auth.js | 2 +- server/swagger.yaml | 44 +++++++++- 12 files changed, 244 insertions(+), 69 deletions(-) create mode 100644 app/src/components/UsernameForm.vue create mode 100644 app/src/views/ChooseUsernamePage.vue diff --git a/app/src/components/AuthenticationPart.vue b/app/src/components/AuthenticationPart.vue index 3f2126a..5ceadf9 100644 --- a/app/src/components/AuthenticationPart.vue +++ b/app/src/components/AuthenticationPart.vue @@ -34,7 +34,7 @@ const handleLogin = async () => { try { const isSuccess = await login(username.value, password.value) if (isSuccess) success() - else errorMessage.value = 'Invalid username or password.' + else errorMessage.value = 'Wrong username or password.' } catch (err) { console.error(err) errorMessage.value = 'Something went wrong.' @@ -47,20 +47,18 @@ async function check() { const res = await api.get('/auth/check-username', { params: { username: user }, }) - // user not exists, try register - const { type } = res.data - if (type === 'email') { - email.value = user - username.value = '' - } - isRegister.value = true - usernameInput.value.focus() - } catch (err) { - if (err.response?.status === 409) { - // user already exists, try login + const { type, available } = res.data + if (available) { + if (type === 'email') { + email.value = user + username.value = '' + } + isRegister.value = true + usernameInput.value.focus() + } else { await handleLogin() - return } + } catch (err) { if (err.response?.status === 400) { errorMessage.value = 'Invalid username or email.' return diff --git a/app/src/components/UsernameForm.vue b/app/src/components/UsernameForm.vue new file mode 100644 index 0000000..05e9e6f --- /dev/null +++ b/app/src/components/UsernameForm.vue @@ -0,0 +1,82 @@ + + diff --git a/app/src/router.js b/app/src/router.js index 40dd4dc..726f439 100644 --- a/app/src/router.js +++ b/app/src/router.js @@ -1,6 +1,7 @@ import { createRouter, createWebHistory } from 'vue-router' import GamePage from './views/GamePage.vue' import AuthPage from './views/AuthPage.vue' +import ChooseUsernamePage from './views/ChooseUsernamePage.vue' import { isLoggedIn } from './utils/auth' const routes = [ @@ -20,6 +21,12 @@ const routes = [ component: GamePage, meta: { requiresAuth: true }, }, + { + path: '/choose-username', + name: 'choose-username', + component: ChooseUsernamePage, + meta: { requiresGuest: true }, + }, ] const router = createRouter({ diff --git a/app/src/views/ChooseUsernamePage.vue b/app/src/views/ChooseUsernamePage.vue new file mode 100644 index 0000000..cec4a14 --- /dev/null +++ b/app/src/views/ChooseUsernamePage.vue @@ -0,0 +1,16 @@ + + diff --git a/server/src/api/controllers/authController.js b/server/src/api/controllers/authController.js index 2d8cf15..acca879 100644 --- a/server/src/api/controllers/authController.js +++ b/server/src/api/controllers/authController.js @@ -16,6 +16,14 @@ export async function issueToken(req, res) { res.json({ token }) } +async function invalidUsername(username) { + return !isValidUsername(username) || User.exists({ username }) +} + +async function invalidEmail(email) { + return !isValidEmail(email) || User.exists({ email }) +} + export async function checkUsername(req, res) { const { username, email } = req.query const cred = username || email @@ -27,30 +35,16 @@ export async function checkUsername(req, res) { } if (isValidEmail(cred)) { - if (await User.exists({ email: cred })) { - res.status(409).json({ - type: 'email', - message: `"${cred}" already exists`, - }) - return - } - res.status(200).json({ + res.json({ type: 'email', - message: `"${cred}" is available`, + available: !(await User.exists({ email: cred })), }) return } if (isValidUsername(cred)) { - if (await User.exists({ username: cred })) { - res.status(409).json({ - type: 'username', - message: `"${cred}" already exists`, - }) - return - } - res.status(200).json({ + res.json({ type: 'username', - message: `"${cred}" is available`, + available: !(await User.exists({ username: cred })), }) return } @@ -68,18 +62,18 @@ export async function register(req, res, next) { return } - if (await User.exists({ username })) { + if (await invalidUsername(username)) { res.status(409).json({ type: 'username', - message: 'username already exists', + message: 'Invalid username', }) return } - if (await User.exists({ email })) { + if (await invalidEmail(email)) { res.status(409).json({ type: 'email', - message: 'email already exists', + message: 'Invalid email', }) return } @@ -126,6 +120,55 @@ export function logout(req, res) { export function checkSession(req, res) { res.json({ - loggedIn: req.isAuthenticated(), + loggedIn: req.isAuthenticated() && !req.session.newUser, + }) +} + +export async function socialLogin(req, res) { + const { newUser } = req.user + if (newUser) { + req.session.newUser = newUser + res.redirect('/choose-username') + return + } + res.redirect('/') +} + +export async function registerFromSession(req, res) { + const { username } = req.body + const { newUser } = req.session + + if (!newUser) { + res.status(400).json({ + message: 'no new user found', + }) + return + } + + if (await invalidUsername(username)) { + res.status(409).json({ + type: 'username', + message: 'Invalid username', + }) + return + } + + newUser.username = username + const user = new User(newUser) + + await user.save() + + req.login(user, (err) => { + if (err) { + console.error(err) + res.status(500).json({ + message: 'error logging in', + }) + return + } + delete req.session.newUser + res.status(201).json({ + message: 'user created', + }) }) } diff --git a/server/src/api/routes/auth.js b/server/src/api/routes/auth.js index 4687610..8b9954d 100644 --- a/server/src/api/routes/auth.js +++ b/server/src/api/routes/auth.js @@ -8,6 +8,8 @@ import { checkUsername, checkSession, logout, + socialLogin, + registerFromSession, } from '../controllers/authController.js' const router = Router() @@ -34,24 +36,14 @@ router.get('/available-methods', (req, res) => { if (enabledMethods.google) { router.get('/google', passport.authenticate('google')) - router.get( - '/google/callback', - passport.authenticate('google', { - successRedirect: '/', - failureRedirect: '/login', - }) - ) + router.get('/google/callback', passport.authenticate('google'), socialLogin) } if (enabledMethods.github) { router.get('/github', passport.authenticate('github')) - router.get( - '/github/callback', - passport.authenticate('github', { - successRedirect: '/', - failureRedirect: '/login', - }) - ) + router.get('/github/callback', passport.authenticate('github'), socialLogin) } +router.post('/register-from-session', registerFromSession) + export default router diff --git a/server/src/auth/githubStrategy.js b/server/src/auth/githubStrategy.js index 7b2120a..ae4e5d7 100644 --- a/server/src/auth/githubStrategy.js +++ b/server/src/auth/githubStrategy.js @@ -5,8 +5,6 @@ import User from '../models/user.js' function verify(accessToken, refreshToken, profile, cb) { process.nextTick(async () => { - console.log(accessToken, refreshToken, profile) - // cb('todo') const email = profile.emails[0]?.value if (!email) { cb(new Error('No email found')) @@ -21,16 +19,18 @@ function verify(accessToken, refreshToken, profile, cb) { displayName: profile.displayName, } if (!user) { - user = new User({ + const newUser = { email, github, - }) - } else if (user.github?.id) { + } + cb(null, { newUser }) + return + } + if (user.github?.id) { cb(new Error('Email already used and binded to another account')) return - } else { - user.github = github } + user.github = github await user.save() } diff --git a/server/src/auth/googleStrategy.js b/server/src/auth/googleStrategy.js index adf27c0..749baa3 100644 --- a/server/src/auth/googleStrategy.js +++ b/server/src/auth/googleStrategy.js @@ -19,16 +19,18 @@ function verify(issuer, profile, cb) { displayName: profile.displayName, } if (!user) { - user = new User({ + const newUser = new User({ email, google, }) - } else if (user.google?.id) { + cb(null, { newUser }) + return + } + if (user.google?.id) { cb(new Error('Email already used and binded to another account')) return - } else { - user.google = google } + user.google = google await user.save() } diff --git a/server/src/middleware/authRequired.js b/server/src/middleware/authRequired.js index 8a5bd4f..df0c937 100644 --- a/server/src/middleware/authRequired.js +++ b/server/src/middleware/authRequired.js @@ -1,5 +1,5 @@ export default function authRequired(req, res, next) { - if (req.isAuthenticated()) { + if (req.isAuthenticated() && !req.session.newUser) { next() return } diff --git a/server/src/models/user.js b/server/src/models/user.js index 5d910c7..3280c3e 100644 --- a/server/src/models/user.js +++ b/server/src/models/user.js @@ -4,8 +4,7 @@ const User = model('User', { username: { type: String, unique: true, - required: false, // should be required, - // but need to let social login users to pick one after login + required: true, }, email: { type: String, diff --git a/server/src/utils/auth.js b/server/src/utils/auth.js index fc8478d..ec95600 100644 --- a/server/src/utils/auth.js +++ b/server/src/utils/auth.js @@ -56,7 +56,7 @@ export function isValidUsername(username) { if (!usernamePattern.test(username)) return false // check if the username is between 1 and 32 characters - if (username.length < 1 || username.length > 32) { + if (username.length < 4 || username.length > 32) { return false } diff --git a/server/swagger.yaml b/server/swagger.yaml index b9f09b3..d52d7bc 100644 --- a/server/swagger.yaml +++ b/server/swagger.yaml @@ -211,6 +211,31 @@ paths: type: string example: User already exists + /auth/register-from-session: + post: + summary: Register a new user from a session, for social login to pick a username + security: + - cookieAuth: [] + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + username: + type: string + format: username + example: alex + responses: + '201': + description: User created + headers: + Set-Cookie: + schema: + type: string + example: connect.sid=s%3Aq1Z; Path=/; HttpOnly; SameSite=Strict; Secure + /auth/login: post: summary: Log in a user @@ -297,19 +322,30 @@ paths: type: string format: username example: alex + - in: query + name: email + description: email to check + required: true + schema: + type: string + format: email + example: alex@example.com responses: '200': - description: Username is available + description: Username / Email checking result content: application/json: schema: type: object properties: - message: + type: type: string example: Username is available - '409': - description: Username is already taken + available: + type: boolean + example: true + '400': + description: Bad Request content: application/json: schema: From d0186727ced8b5707d07c7fa80f5ebd9c0e055ee Mon Sep 17 00:00:00 2001 From: wancat Date: Fri, 13 Oct 2023 00:18:56 +0800 Subject: [PATCH 2/8] feat: add chatgpt code reivew --- .github/workflows/code_review.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .github/workflows/code_review.yml diff --git a/.github/workflows/code_review.yml b/.github/workflows/code_review.yml new file mode 100644 index 0000000..9a54622 --- /dev/null +++ b/.github/workflows/code_review.yml @@ -0,0 +1,15 @@ +name: CI/CD + +on: + pull_request: + types: [opened, synchronize, reopened] + +jobs: + gpt-review: + runs-on: ubuntu-latest + + steps: + - name: gpt-review + uses: microsoft/gpt-review@v0.9.5 + with: + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} From db0126de53684b07c10cadbc41d7b23af4669248 Mon Sep 17 00:00:00 2001 From: wancat Date: Fri, 13 Oct 2023 00:25:14 +0800 Subject: [PATCH 3/8] fix: change to use mattzcarey/code-review-gpt --- .github/workflows/code_review.yml | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/.github/workflows/code_review.yml b/.github/workflows/code_review.yml index 9a54622..e482cb9 100644 --- a/.github/workflows/code_review.yml +++ b/.github/workflows/code_review.yml @@ -1,15 +1,20 @@ -name: CI/CD +name: Code Review GPT on: pull_request: - types: [opened, synchronize, reopened] + branches: [main] jobs: - gpt-review: + run_code_review: runs-on: ubuntu-latest - steps: - - name: gpt-review - uses: microsoft/gpt-review@v0.9.5 + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - name: Code Review GPT + uses: mattzcarey/code-review-gpt@v0.1.4-alpha with: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + MODEL: 'gpt-3.5-turbo' + GITHUB_TOKEN: ${{ github.token }} From 9253feb52026b481ff03854c0a4a4813030cfb62 Mon Sep 17 00:00:00 2001 From: wancat Date: Fri, 13 Oct 2023 00:32:58 +0800 Subject: [PATCH 4/8] fix: add permission --- .github/workflows/code_review.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/code_review.yml b/.github/workflows/code_review.yml index e482cb9..e15258e 100644 --- a/.github/workflows/code_review.yml +++ b/.github/workflows/code_review.yml @@ -6,6 +6,8 @@ on: jobs: run_code_review: + permissions: + pull-requests: write runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 From 7a9206fb47f16af91a90cd61c6c61cf500b1b177 Mon Sep 17 00:00:00 2001 From: wancat Date: Fri, 13 Oct 2023 00:51:00 +0800 Subject: [PATCH 5/8] refactor: reuse oauthVerify function --- server/src/auth/githubStrategy.js | 35 ++----------------------------- server/src/auth/googleStrategy.js | 35 ++----------------------------- server/src/auth/oauthVerify.js | 24 +++++++++++++++++++++ 3 files changed, 28 insertions(+), 66 deletions(-) create mode 100644 server/src/auth/oauthVerify.js diff --git a/server/src/auth/githubStrategy.js b/server/src/auth/githubStrategy.js index ae4e5d7..d3af989 100644 --- a/server/src/auth/githubStrategy.js +++ b/server/src/auth/githubStrategy.js @@ -1,41 +1,10 @@ import GitHubStrategy from 'passport-github2' import config from '../config.js' -import User from '../models/user.js' +import oauthVerify from './oauthVerify.js' function verify(accessToken, refreshToken, profile, cb) { - process.nextTick(async () => { - const email = profile.emails[0]?.value - if (!email) { - cb(new Error('No email found')) - return - } - - let user = await User.findOne({ 'github.id': profile.id }) - if (!user) { - user = await User.findOne({ email }) - const github = { - id: profile.id, - displayName: profile.displayName, - } - if (!user) { - const newUser = { - email, - github, - } - cb(null, { newUser }) - return - } - if (user.github?.id) { - cb(new Error('Email already used and binded to another account')) - return - } - user.github = github - await user.save() - } - - cb(null, user) - }) + oauthVerify('github', profile, { 'github.id': profile.id }, cb) } const enabled = config.github.clientID && config.github.clientSecret diff --git a/server/src/auth/googleStrategy.js b/server/src/auth/googleStrategy.js index 749baa3..566a09e 100644 --- a/server/src/auth/googleStrategy.js +++ b/server/src/auth/googleStrategy.js @@ -1,41 +1,10 @@ import GoogleStrategy from 'passport-google-oidc' import config from '../config.js' -import User from '../models/user.js' +import oauthVerify from './oauthVerify.js' function verify(issuer, profile, cb) { - process.nextTick(async () => { - const email = profile.emails[0]?.value - if (!email) { - cb(new Error('No email found')) - return - } - - let user = await User.findOne({ 'google.id': profile.id }) - if (!user) { - user = await User.findOne({ email }) - const google = { - id: profile.id, - displayName: profile.displayName, - } - if (!user) { - const newUser = new User({ - email, - google, - }) - cb(null, { newUser }) - return - } - if (user.google?.id) { - cb(new Error('Email already used and binded to another account')) - return - } - user.google = google - await user.save() - } - - cb(null, user) - }) + oauthVerify('google', profile, { 'google.id': profile.id }, cb) } const enabled = config.google.clientID && config.google.clientSecret diff --git a/server/src/auth/oauthVerify.js b/server/src/auth/oauthVerify.js new file mode 100644 index 0000000..55b95ee --- /dev/null +++ b/server/src/auth/oauthVerify.js @@ -0,0 +1,24 @@ +import User from '../models/user.js' + +export default async function oauthVerify(provider, profile, query, cb) { + process.nextTick(async () => { + const email = profile.emails[0]?.value + if (!email) { + return cb(new Error('No email found')) + } + + let user = await User.findOne(query) + if (!user) { + user = await User.findOne({ email }) + if (!user) { + const newUser = { email } + newUser[provider] = profile + return cb(null, { newUser }) + } + user[provider] = profile + await user.save() + } + + return cb(null, user) + }) +} From 30aeacaf0678a0355dced56ae80bc50987b952fa Mon Sep 17 00:00:00 2001 From: "Lin, Hung-Shin" Date: Fri, 13 Oct 2023 16:13:25 +0800 Subject: [PATCH 6/8] feat: separate Login and Register Page --- .../{AuthenticationPart.vue => AuthForm.vue} | 156 +++++++----------- app/src/components/HeaderPart.vue | 2 +- app/src/router.js | 15 +- app/src/views/AuthPage.vue | 16 -- app/src/views/LoginPage.vue | 28 ++++ app/src/views/RegisterPage.vue | 56 +++++++ 6 files changed, 159 insertions(+), 114 deletions(-) rename app/src/components/{AuthenticationPart.vue => AuthForm.vue} (60%) delete mode 100644 app/src/views/AuthPage.vue create mode 100644 app/src/views/LoginPage.vue create mode 100644 app/src/views/RegisterPage.vue diff --git a/app/src/components/AuthenticationPart.vue b/app/src/components/AuthForm.vue similarity index 60% rename from app/src/components/AuthenticationPart.vue rename to app/src/components/AuthForm.vue index 5ceadf9..c37fae5 100644 --- a/app/src/components/AuthenticationPart.vue +++ b/app/src/components/AuthForm.vue @@ -1,15 +1,18 @@