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.

Sign up for GitHub

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

Jump to bottom

Github SSO #191

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Github SSO #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2496449
enable github button
MrSunshyne Aug 16, 2024
a22fdbf
add github auth function
MrSunshyne Aug 16, 2024
7fdee05
chore: Update redirect URL for GitHub authentication
MrSunshyne Aug 16, 2024
d421b4a
add discord
MrSunshyne Aug 16, 2024
066a1d0
deploy mode cookie
MrSunshyne Sep 2, 2024
8686920
add provider
MrSunshyne Sep 2, 2024
16f0fe1
Revert "add provider"
MrSunshyne Sep 2, 2024
11a4995
request provider for self
MrSunshyne Sep 2, 2024
8e5b772
show profile pic for github users
MrSunshyne Sep 2, 2024
f4b7d4c
display user (yes we are debugging in prod)
MrSunshyne Sep 2, 2024
25f9d17
add provider and external identifier to valid user
MrSunshyne Sep 2, 2024
53e7287
hide discord
MrSunshyne Sep 2, 2024
98ff8be
refactor: Improve user full name handling in mapToValidUser function
MrSunshyne Sep 2, 2024
04593b1
feat: Add provider hint to LoggedUser component
MrSunshyne Sep 2, 2024
1c721a0
feat: Add provider hint to profile
MrSunshyne Sep 2, 2024
fa83d9f
Merge branch 'main' into 163-github-login
MrSunshyne Oct 9, 2024
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
23 changes: 20 additions & 3 deletions packages/frontendmu-nuxt/auth-utils/useAuth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export default function useAuth(client: DirectusClient<any> & AuthenticationClie
credentials: 'include', // this is required in order to send the refresh token cookie
body: JSON.stringify({
refresh_token: getCookieValue('directus_session_token'),
mode: 'cookie',
}),
},
)
Expand Down Expand Up @@ -171,10 +172,12 @@ export default function useAuth(client: DirectusClient<any> & AuthenticationClie
'meal',
'occupation',
'github_username',
'external_identifier',
'Events.Events_id.id',
'Events.Events_id.title',
'profile_picture',
'role.name',
'provider',
]

try {
Expand Down Expand Up @@ -220,9 +223,23 @@ export default function useAuth(client: DirectusClient<any> & AuthenticationClie
return false
})

function oAuthLogin() {
const currentPage = new URL(window.location.origin)
return `${DIRECTUS_URL()}/auth/login/google?redirect=${currentPage}redirect`
function oAuthLogin(provider: string) {
if (provider === 'google') {
const currentPage = new URL(window.location.origin)
return `${DIRECTUS_URL()}/auth/login/google?redirect=${currentPage}redirect`
}
else if (provider === 'github') {
const currentPage = new URL(window.location.origin)
return `${DIRECTUS_URL()}/auth/login/github?redirect=${currentPage}redirect`
}
else if (provider === 'discord') {
const currentPage = new URL(window.location.origin)
return `${DIRECTUS_URL()}/auth/login/discord?redirect=${currentPage}redirect`
}

console.log('Provider not found')

return false
Comment on lines +226 to +242
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor 'oAuthLogin' function to eliminate code duplication

The oAuthLogin function contains repetitive code for each provider. Consider refactoring to eliminate duplication and improve maintainability. You can use a mapping of providers to their respective paths and handle the redirect logic more concisely.

Apply this diff to refactor the function:

      function oAuthLogin(provider: string) {
-        if (provider === 'google') {
-          const currentPage = new URL(window.location.origin)
-          return `${DIRECTUS_URL()}/auth/login/google?redirect=${currentPage}redirect`
-        }
-        else if (provider === 'github') {
-          const currentPage = new URL(window.location.origin)
-          return `${DIRECTUS_URL()}/auth/login/github?redirect=${currentPage}redirect`
-        }
-        else if (provider === 'discord') {
-          const currentPage = new URL(window.location.origin)
-          return `${DIRECTUS_URL()}/auth/login/discord?redirect=${currentPage}redirect`
-        }
-
-        console.log('Provider not found')
-
-        return false
+        const currentPage = window.location.origin + '/redirect'
+        const providers = ['google', 'github', 'discord']
+        if (providers.includes(provider)) {
+          return `${DIRECTUS_URL()}/auth/login/${provider}?redirect=${currentPage}`
+        } else {
+          console.error('Provider not found')
+          return false
+        }
      }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function oAuthLogin(provider: string) {
if (provider === 'google') {
const currentPage = new URL(window.location.origin)
return `${DIRECTUS_URL()}/auth/login/google?redirect=${currentPage}redirect`
}
else if (provider === 'github') {
const currentPage = new URL(window.location.origin)
return `${DIRECTUS_URL()}/auth/login/github?redirect=${currentPage}redirect`
}
else if (provider === 'discord') {
const currentPage = new URL(window.location.origin)
return `${DIRECTUS_URL()}/auth/login/discord?redirect=${currentPage}redirect`
}
console.log('Provider not found')
return false
function oAuthLogin(provider: string) {
const currentPage = window.location.origin + '/redirect'
const providers = ['google', 'github', 'discord']
if (providers.includes(provider)) {
return `${DIRECTUS_URL()}/auth/login/${provider}?redirect=${currentPage}`
} else {
console.error('Provider not found')
return false
}
}

}

async function updateUserProfile({ profile_updates }: { profile_updates: DirectusAstroUser }) {
Expand Down
71 changes: 58 additions & 13 deletions packages/frontendmu-nuxt/components/auth/LoggedUser.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,34 +16,74 @@ onMounted(() => {
<template>
<div class="dark:text-zinc-200 dark:ring-white/10 pl-4">
<BaseButton
v-if="!isLoggedIn" href="/login" color="primary" class="font-medium hidden md:block"
v-if="!isLoggedIn"
href="/login"
color="primary"
class="font-medium hidden md:block"
@click="setUrl()"
>
Log In
</BaseButton>
<BaseButton
v-if="!isLoggedIn" href="/login" color="primary" class="font-medium block md:hidden" size="sm"
v-if="!isLoggedIn"
href="/login"
color="primary"
class="font-medium block md:hidden"
size="sm"
@click="setUrl()"
>
Log In
</BaseButton>
<div v-else class="flex gap-2 items-center">
<Menu as="div" class="relative inline-block text-left">
<div
v-else
class="flex gap-2 items-center"
>
<Menu
as="div"
class="relative inline-block text-left"
>
<div>
<MenuButton
class="inline-flex items-center w-full justify-center gap-x-1.5 rounded-full text-sm font-semibold text-verse-900 dark:text-verse-100 shadow-sm ring-gray-300 hover:bg-gray-50"
class="inline-flex relative items-center w-full justify-center gap-x-1.5 rounded-full text-sm font-semibold text-verse-900 dark:text-verse-100 shadow-sm ring-gray-300 hover:bg-gray-50"
>
<div v-if="avatarUrl">
<img class="w-10 aspect-square rounded-full" :src="avatarUrl">
<template v-if="user?.provider === 'github'">
<img
class="w-10 aspect-square rounded-full"
:src="`https://github.com/${user?.external_identifier}.png`"
>
</template>
<template v-else>
<div v-if="avatarUrl">
<img
class="w-10 aspect-square rounded-full"
:src="avatarUrl"
>
</div>
</template>
Comment on lines +47 to +62
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

GitHub SSO avatar handling implemented correctly, but error cases need attention

The codebase analysis confirms that the GitHub SSO avatar handling has been implemented as intended. The LoggedUser.vue component correctly renders the GitHub avatar using the external_identifier:

<template v-if="user?.provider === 'github'">
  <img
    class="w-10 aspect-square rounded-full"
    :src="`https://github.com/${user?.external_identifier}.png`"
  >
</template>

However, the initial concern about error handling remains valid. To address potential error cases, consider the following recommendations:

  1. Implement a fallback avatar for cases where the GitHub avatar URL is invalid or inaccessible.
  2. Add error handling for scenarios where the GitHub sign-in fails or the user's email is not set to public in their GitHub settings.
  3. In the useAuth.ts file, enhance the avatarUrl computed property to handle potential errors:
const avatarUrl = computed(() => {
  if (user.value?.avatar_url) {
    return user.value.avatar_url
  } else if (rawUser.value?.github_username) {
    return `https://github.com/${rawUser.value.github_username}.png`
  } else if (user.value?.provider === 'github' && user.value?.external_identifier) {
    return `https://github.com/${user.value.external_identifier}.png`
  } else {
    return '/path/to/fallback/avatar.png' // Provide a fallback avatar
  }
})
  1. In the LoggedUser.vue component, add error handling for the avatar image:
<img
  class="w-10 aspect-square rounded-full"
  :src="`https://github.com/${user?.external_identifier}.png`"
  @error="handleAvatarError"
  alt="User avatar"
>

Add a method to handle avatar loading errors:

function handleAvatarError(event: Event) {
  const img = event.target as HTMLImageElement;
  img.src = '/path/to/fallback/avatar.png'; // Set a fallback avatar
}

These changes will improve the robustness of the avatar handling and provide a better user experience in case of errors.

🔗 Analysis chain

GitHub SSO avatar handling implemented, but error cases need attention

The addition of conditional rendering for user avatars, especially the GitHub-specific handling, is a good step towards implementing the GitHub SSO functionality. However, as per the PR objectives, we need to address potential error cases.

Consider adding error handling for scenarios where:

  1. The GitHub sign-in fails
  2. The user's email is not set to public in their GitHub settings
  3. The avatar URL is invalid or inaccessible

To ensure we're handling all potential avatar sources correctly, let's verify the user object structure:

Correction Needed: Shell script errors detected during user object verification

The previous verification script encountered errors due to unrecognized file types and a typo in the directory path. Please use the corrected script below to verify the structure of the user object:

Further Verification Needed: Unable to locate user object definitions

The previous attempts to verify the user object structure did not yield any results. To ensure comprehensive coverage, please execute the following scripts to search for any occurrences of user and its declarations within the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the structure of the user object in auth-related files

# Test: Search for user object definitions or usages
rg --type vue --type ts 'user\s*:\s*\{' packages/frontendmu-nuxt

Length of output: 197

Script:

# Description: Verify the structure of the user object in auth-related files

# Corrected Test: Search for user object definitions or usages in .vue and .ts files
rg 'user\s*:\s*\{' packages/frontendmu-nuxt --glob '*.vue' --glob '*.ts'

Length of output: 74

Script:

# Description: Broad search for any occurrences of 'user' in .vue and .ts files

rg 'user' packages/frontendmu-nuxt --glob '*.vue' --glob '*.ts'

Length of output: 17940

Script:

# Description: Use ast-grep to find user object declarations in .vue and .ts files

ast-grep --lang typescript --pattern $'const user: $_Type = { $$$ }' packages/frontendmu-nuxt

Length of output: 94

<!-- Provider Hint -->
<div class="absolute bottom-0 right-0 bg-white rounded-full">
<div v-if="user?.provider === 'default'">
<Icon
class="block w-3 h-3 p-0.5 text-black"
name="fluent:animal-rabbit-24-regular"
/>
</div>
<div v-else>
<Icon
:name="`carbon:logo-${user?.provider}`"
class="block w-3 h-3 p-0.5 text-black"
/>
</div>
</div>
<!-- <ChevronDownIcon class="-mr-1 h-5 w-5 text-gray-400" aria-hidden="true" /> -->
</MenuButton>
</div>

<transition
enter-active-class="transition ease-out duration-100"
enter-from-class="transform opacity-0 scale-95" enter-to-class="transform opacity-100 scale-100"
leave-active-class="transition ease-in duration-75" leave-from-class="transform opacity-100 scale-100"
enter-from-class="transform opacity-0 scale-95"
enter-to-class="transform opacity-100 scale-100"
leave-active-class="transition ease-in duration-75"
leave-from-class="transform opacity-100 scale-100"
leave-to-class="transform opacity-0 scale-95"
>
<MenuItems
Expand All @@ -53,15 +93,17 @@ onMounted(() => {
<MenuItem v-slot="{ active }">
<NuxtLink
href="/user/me"
class="block px-4 py-2 text-sm" :class="[active ? 'bg-gray-400/10 text-verse-900 dark:text-verse-100' : 'text-verse-900 dark:text-verse-100']"
class="block px-4 py-2 text-sm"
:class="[active ? 'bg-gray-400/10 text-verse-900 dark:text-verse-100' : 'text-verse-900 dark:text-verse-100']"
>
My
Profile
</NuxtLink>
</MenuItem>
<MenuItem v-slot="{ active }">
<NuxtLink
class="block px-4 py-2 text-sm cursor-pointer" :class="[active ? 'bg-gray-400/10 text-verse-900 dark:text-verse-100' : 'text-verse-900 dark:text-verse-100']"
class="block px-4 py-2 text-sm cursor-pointer"
:class="[active ? 'bg-gray-400/10 text-verse-900 dark:text-verse-100' : 'text-verse-900 dark:text-verse-100']"
@click="logout()"
>
Logout
Expand All @@ -71,7 +113,10 @@ onMounted(() => {
<div class="text-verse-900 dark:text-verse-100 p-4 text-sm">
<div class="flex flex-col items-center justify-center gap-2">
<div v-if="avatarUrl">
<img class="w-16 aspect-square rounded-full" :src="avatarUrl">
<img
class="w-16 aspect-square rounded-full"
:src="avatarUrl"
>
</div>

{{ user?.full_name }}
Expand Down
63 changes: 51 additions & 12 deletions packages/frontendmu-nuxt/components/auth/LoginForm.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,36 @@ const developmentEnvironment = process.env.NODE_ENV === 'development'

<template>
<div class="sm:mx-auto sm:w-full sm:max-w-md">
<MiscLogoFec class="w-10 mx-auto dark:text-white" :loading="isLoading" />
<MiscLogoFec
class="w-10 mx-auto dark:text-white"
:loading="isLoading"
/>
<h2 class="mt-6 text-center text-2xl font-bold leading-9 tracking-tight text-verse-900 dark:text-verse-100">
{{ isLoggedIn ? 'Welcome back' : 'Sign in to your account' }}
</h2>
</div>
<div class="mt-10 sm:mx-auto sm:w-full sm:max-w-[480px] flex justify-center">
<div class="bg-verse-200/10 overflow-hidden dark:bg-verse-500/20 backdrop-blur-sm px-6 py-12 shadow-2xl rounded-lg sm:px-12 w-11/12">
<div
class="bg-verse-200/10 overflow-hidden dark:bg-verse-500/20 backdrop-blur-sm px-6 py-12 shadow-2xl rounded-lg sm:px-12 w-11/12"
>
<div v-if="isLoggedIn">
<div class="text-center flex flex-col gap-8 text-verse-900 dark:text-verse-100 w-full">
<span>
You are logged in as <NuxtLink class="underline" href="/user/me"> {{ user?.full_name }}
You are logged in as <NuxtLink
class="underline"
href="/user/me"
> {{ user?.full_name }}
</NuxtLink>
</span>
<div v-if="willRedirect">
redirecting in {{ Math.round(countdown / 1000) }}s
</div>

<div class="grid grid-cols-2 gap-4 w-full justify-evenly">
<BaseButton color="neutral" @click="logout">
<BaseButton
color="neutral"
@click="logout"
>
Logout
</BaseButton>
<BaseButton href="/user/me">
Expand All @@ -50,7 +61,11 @@ const developmentEnvironment = process.env.NODE_ENV === 'development'
</div>
</div>
<div v-else>
<form v-if="developmentEnvironment" class="space-y-6" @submit.prevent="login()">
<form
v-if="developmentEnvironment"
class="space-y-6"
@submit.prevent="login()"
>
<div>
<label
for="email"
Expand Down Expand Up @@ -84,7 +99,8 @@ const developmentEnvironment = process.env.NODE_ENV === 'development'
v-model="password"
name="password"
type="password"
autocomplete="current-password" required
autocomplete="current-password"
required
class="block w-full rounded-md border-0 p-1.5 text-verse-900 shadow-sm ring-1 ring-inset ring-gray-300 placeholder:text-gray-400 focus:ring-2 focus:ring-inset focus:ring-verse-600 sm:text-sm sm:leading-6"
>
</div>
Expand Down Expand Up @@ -118,26 +134,37 @@ const developmentEnvironment = process.env.NODE_ENV === 'development'
</div>

<div class="relative">
<div class="absolute inset-0 flex items-center" aria-hidden="true">
<div
class="absolute inset-0 flex items-center"
aria-hidden="true"
>
<div class="w-full border-t border-verse-500/20" />
</div>
</div>

<div class="mt-10 grid sm:grid-cols-2 gap-4">
<a
:href="oAuthLogin()"
:href="oAuthLogin('google')"
class="flex w-full items-center justify-center gap-3 rounded-md bg-[#000000] hover:bg-black/50 transition-all duration-300 px-3 py-1.5 text-white focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[#1D9BF0]"
>
<Icon name="logos:google-icon" class="h-5 w-5" />
<Icon
name="logos:google-icon"
class="h-5 w-5"
/>
<span class="text-sm font-semibold leading-6">Google</span>
</a>

<a
href="#"
class="opacity-60 cursor-not-allowed flex w-full items-center justify-center gap-3 rounded-md bg-[#24292F] px-3 py-1.5 text-white focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[#24292F]"
:href="oAuthLogin('github')"
class="flex w-full items-center justify-center gap-3 rounded-md bg-[#000000] hover:bg-black/50 px-3 py-1.5 text-white focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[#24292F]"
disabled
>
<svg class="h-5 w-5" aria-hidden="true" fill="currentColor" viewBox="0 0 20 20">
<svg
class="h-5 w-5"
aria-hidden="true"
fill="currentColor"
viewBox="0 0 20 20"
>
<path
fill-rule="evenodd"
d="M10 0C4.477 0 0 4.484 0 10.017c0 4.425 2.865 8.18 6.839 9.504.5.092.682-.217.682-.483 0-.237-.008-.868-.013-1.703-2.782.605-3.369-1.343-3.369-1.343-.454-1.158-1.11-1.466-1.11-1.466-.908-.62.069-.608.069-.608 1.003.07 1.531 1.032 1.531 1.032.892 1.53 2.341 1.088 2.91.832.092-.647.35-1.088.636-1.338-2.22-.253-4.555-1.113-4.555-4.951 0-1.093.39-1.988 1.029-2.688-.103-.253-.446-1.272.098-2.65 0 0 .84-.27 2.75 1.026A9.564 9.564 0 0110 4.844c.85.004 1.705.115 2.504.337 1.909-1.296 2.747-1.027 2.747-1.027.546 1.379.203 2.398.1 2.651.64.7 1.028 1.595 1.028 2.688 0 3.848-2.339 4.695-4.566 4.942.359.31.678.921.678 1.856 0 1.338-.012 2.419-.012 2.747 0 .268.18.58.688.482A10.019 10.019 0 0020 10.017C20 4.484 15.522 0 10 0z"
Expand All @@ -146,6 +173,18 @@ const developmentEnvironment = process.env.NODE_ENV === 'development'
</svg>
<span class="text-sm font-semibold leading-6">GitHub</span>
</a>

<!-- <a
:href="oAuthLogin('discord')"
class="flex w-full items-center justify-center gap-3 rounded-md bg-[#000000] hover:bg-black/50 px-3 py-1.5 text-white focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[#24292F]"
disabled
>
<Icon
name="logos:discord-icon"
class="h-5 w-5"
/>
<span class="text-sm font-semibold leading-6">Discord</span>
</a> -->
</div>
</div>
</div>
Expand Down
Loading
Loading