Skip to content

add {PROVIDER}_DISPLAY_NAME env-var #8967

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions packages/account-client/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
type PersonId,
type PersonInfo,
type PersonUuid,
type ProviderInfo,
type SocialIdType,
Version,
type WorkspaceInfoWithStatus,
Expand Down Expand Up @@ -52,7 +53,7 @@ import { getClientTimezone } from './utils'
/** @public */
export interface AccountClient {
// Static methods
getProviders: () => Promise<string[]>
getProviders: () => Promise<ProviderInfo[]>

// RPC
getUserWorkspaces: () => Promise<WorkspaceInfoWithStatus[]>
Expand Down Expand Up @@ -217,7 +218,7 @@ class AccountClientImpl implements AccountClient {
this.rpc = withRetryUntilTimeout(this._rpc.bind(this), retryTimeoutMs ?? 5000)
}

async getProviders (): Promise<string[]> {
async getProviders (): Promise<ProviderInfo[]> {
return await withRetryUntilMaxAttempts(async () => {
const response = await fetch(concatLink(this.url, '/providers'))

Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/classes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -869,4 +869,9 @@ export interface AccountInfo {
locale?: string
}

export interface ProviderInfo {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feels like it doesn't belong to core. Please define it both in auth-providers and in account-client instead.

name: string
displayName?: string
}

export type SocialKey = Pick<SocialId, 'type' | 'value'>
27 changes: 20 additions & 7 deletions plugins/login-resources/src/components/Providers.svelte
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script lang="ts">
import { concatLink } from '@hcengineering/core'
import { concatLink, type ProviderInfo } from '@hcengineering/core'
import { getMetadata } from '@hcengineering/platform'
import { AnySvelteComponent, Button, Grid, deviceOptionsStore, getCurrentLocation } from '@hcengineering/ui'
import { onMount } from 'svelte'
Expand All @@ -12,28 +12,41 @@
interface Provider {
name: string
component: AnySvelteComponent
displayName: null
}

const providers: Provider[] = [
{
name: 'google',
component: Google
component: Google,
displayName: null
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: nulls looks strange here. Please consider replacing this structure with a simple mapping of name -> component and only use Provider type for enabledProviders.

},
{
name: 'github',
component: Github
component: Github,
displayName: null
},
{
name: 'openid',
component: OpenId
component: OpenId,
displayName: null
}
]

let enabledProviders: Provider[] = []

onMount(() => {
void getProviders().then((res) => {
enabledProviders = providers.filter((provider) => res.includes(provider.name))
void getProviders().then((res: ProviderInfo[]) => {
enabledProviders = providers
.map((provider) => {
const match = res.find((r) => r.name === provider.name)
if (!match) return null
return {
...provider,
displayName: match.displayName
}
})
.filter((p): p is Provider & { displayName: string } => p !== null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: there's a notEmpty utility which can simplify this line to filter(notEmpty)

})
})

Expand Down Expand Up @@ -70,7 +83,7 @@
<a href={getLink(provider)}>
<Button kind={'contrast'} shape={'round2'} size={'x-large'} width="100%" stopPropagation={false}>
<svelte:fragment slot="content">
<svelte:component this={provider.component} />
<svelte:component this={provider.component} displayName={provider.displayName} />
</svelte:fragment>
</Button>
</a>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
import { Label } from '@hcengineering/ui'
import Github from '../icons/Github.svelte'
import login from '../../plugin'

export let displayName = 'Github'
</script>

<div class="flex-row-center flex-gap-2">
<Github />
<Label label={login.string.ContinueWith} params={{ provider: 'Github' }} />
<Label label={login.string.ContinueWith} params={{ provider: displayName }} />
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
import { Label } from '@hcengineering/ui'
import Google from '../icons/Google.svelte'
import login from '../../plugin'

export let displayName = 'Google'
</script>

<div class="flex-row-center flex-gap-2">
<Google />
<Label label={login.string.ContinueWith} params={{ provider: 'Google' }} />
<Label label={login.string.ContinueWith} params={{ provider: displayName }} />
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
import { Label } from '@hcengineering/ui'
import OpenId from '../icons/OpenId.svelte'
import login from '../../plugin'

export let displayName = 'OpenId'
</script>

<div class="flex-row-center flex-gap-2">
<OpenId />
<Label label={login.string.ContinueWith} params={{ provider: 'OpenId' }} />
<Label label={login.string.ContinueWith} params={{ provider: displayName }} />
</div>
7 changes: 4 additions & 3 deletions plugins/login-resources/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ import {
type AccountUuid,
type Person,
type WorkspaceInfoWithStatus,
type WorkspaceUserOperation
type WorkspaceUserOperation,
type ProviderInfo
} from '@hcengineering/core'
import { loginId } from '@hcengineering/login'
import platform, {
Expand Down Expand Up @@ -851,8 +852,8 @@ export async function getLoginInfoFromQuery (): Promise<LoginInfo | WorkspaceLog
}
}

export async function getProviders (): Promise<string[]> {
let providers: string[]
export async function getProviders (): Promise<ProviderInfo[]> {
let providers: ProviderInfo[]

try {
providers = await getAccountClient(null).getProviders()
Expand Down
7 changes: 5 additions & 2 deletions pods/authProviders/src/github.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { type AccountDB } from '@hcengineering/account'
import { type ProviderInfo } from '@hcengineering/core'
import { BrandingMap, concatLink, MeasureContext, getBranding, SocialIdType } from '@hcengineering/core'
import Router from 'koa-router'
import { Strategy as GitHubStrategy } from 'passport-github2'
Expand All @@ -14,9 +15,11 @@ export function registerGithub (
frontUrl: string,
brandings: BrandingMap,
signUpDisabled?: boolean
): string | undefined {
): ProviderInfo | undefined {
const GITHUB_CLIENT_ID = process.env.GITHUB_CLIENT_ID
const GITHUB_CLIENT_SECRET = process.env.GITHUB_CLIENT_SECRET
const name = 'github'
const displayName = process.env.GITHUB_DISPLAY_NAME

const redirectURL = '/auth/github/callback'
if (GITHUB_CLIENT_ID === undefined || GITHUB_CLIENT_SECRET === undefined) return
Expand Down Expand Up @@ -80,5 +83,5 @@ export function registerGithub (
}
)

return 'github'
return { name, displayName }
}
7 changes: 5 additions & 2 deletions pods/authProviders/src/google.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { type AccountDB } from '@hcengineering/account'
import { type ProviderInfo } from '@hcengineering/core'
import { BrandingMap, concatLink, MeasureContext, getBranding, SocialIdType } from '@hcengineering/core'
import Router from 'koa-router'
import { Strategy as GoogleStrategy } from 'passport-google-oauth20'
Expand All @@ -14,9 +15,11 @@ export function registerGoogle (
frontUrl: string,
brandings: BrandingMap,
signUpDisabled?: boolean
): string | undefined {
): ProviderInfo | undefined {
const GOOGLE_CLIENT_ID = process.env.GOOGLE_CLIENT_ID
const GOOGLE_CLIENT_SECRET = process.env.GOOGLE_CLIENT_SECRET
const name = 'google'
const displayName = process.env.GOOGLE_DISPLAY_NAME

const redirectURL = '/auth/google/callback'
if (GOOGLE_CLIENT_ID === undefined || GOOGLE_CLIENT_SECRET === undefined) return
Expand Down Expand Up @@ -85,5 +88,5 @@ export function registerGoogle (
}
)

return 'google'
return { name, displayName }
}
6 changes: 3 additions & 3 deletions pods/authProviders/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { registerGithub } from './github'
import { registerGoogle } from './google'
import { registerOpenid } from './openid'
import { registerToken } from './token'
import { BrandingMap, MeasureContext } from '@hcengineering/core'
import { BrandingMap, MeasureContext, type ProviderInfo } from '@hcengineering/core'
import { type AccountDB } from '@hcengineering/account'

export type Passport = typeof passport
Expand All @@ -20,7 +20,7 @@ export type AuthProvider = (
frontUrl: string,
brandings: BrandingMap,
signUpDisabled?: boolean
) => string | undefined
) => ProviderInfo | undefined

export function registerProviders (
ctx: MeasureContext,
Expand Down Expand Up @@ -62,7 +62,7 @@ export function registerProviders (

registerToken(ctx, passport, router, accountsUrl, db, frontUrl, brandings)

const res: string[] = []
const res: ProviderInfo[] = []
const providers: AuthProvider[] = [registerGoogle, registerGithub, registerOpenid]
for (const provider of providers) {
const value = provider(ctx, passport, router, accountsUrl, db, frontUrl, brandings, signUpDisabled)
Expand Down
7 changes: 5 additions & 2 deletions pods/authProviders/src/openid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.
//
import { type AccountDB } from '@hcengineering/account'
import { type ProviderInfo } from '@hcengineering/core'
import { BrandingMap, concatLink, MeasureContext, getBranding, SocialIdType } from '@hcengineering/core'
import Router from 'koa-router'
import { Issuer, Strategy } from 'openid-client'
Expand All @@ -29,10 +30,12 @@ export function registerOpenid (
frontUrl: string,
brandings: BrandingMap,
signUpDisabled?: boolean
): string | undefined {
): ProviderInfo | undefined {
const openidClientId = process.env.OPENID_CLIENT_ID
const openidClientSecret = process.env.OPENID_CLIENT_SECRET
const issuer = process.env.OPENID_ISSUER
const name = 'openid'
const displayName = process.env.OPENID_DISPLAY_NAME

const redirectURL = '/auth/openid/callback'
if (openidClientId === undefined || openidClientSecret === undefined || issuer === undefined) return
Expand Down Expand Up @@ -110,5 +113,5 @@ export function registerOpenid (
}
)

return 'openid'
return { name, displayName }
}