Skip to content

Commit

Permalink
fix: Refresh the SAML request URL for each login attempt (#10593)
Browse files Browse the repository at this point in the history
  • Loading branch information
Dschoordsch authored Dec 13, 2024
1 parent dbe20eb commit 22d89e5
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 21 deletions.
18 changes: 14 additions & 4 deletions packages/client/Atmosphere.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
ConcreteRequest,
Environment,
FetchFunction,
FetchQueryFetchPolicy,
GraphQLResponse,
GraphQLTaggedNode,
Network,
Expand Down Expand Up @@ -340,13 +341,22 @@ export default class Atmosphere extends Environment {

fetchQuery = async <T extends OperationType>(
taggedNode: GraphQLTaggedNode,
variables: Variables = {}
variables: Variables = {},
cacheConfig?: {
networkCacheConfig?: CacheConfig
fetchPolicy?: FetchQueryFetchPolicy
}
) => {
let res: T['response']
try {
res = await fetchQuery<T>(this, taggedNode, variables, {
fetchPolicy: 'store-or-network'
}).toPromise()
res = await fetchQuery<T>(
this,
taggedNode,
variables,
cacheConfig ?? {
fetchPolicy: 'store-or-network'
}
).toPromise()
} catch (e) {
return null
}
Expand Down
14 changes: 7 additions & 7 deletions packages/client/components/EmailPasswordAuthForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@ const EmailPasswordAuthForm = forwardRef((props: Props, ref: any) => {
const signInWithSSOOnly = isSSOAuthEnabled && !isInternalAuthEnabled
const [isSSO, setIsSSO] = useState(isSSODefault || signInWithSSOOnly)
const [pendingDomain, setPendingDomain] = useState('')
const [ssoURL, setSSOURL] = useState('')
const [ssoDomain, setSSODomain] = useState('')
const [ssoDomain, setSSODomain] = useState<string>()
const {submitMutation, onCompleted, submitting, error, onError} = useMutationProps()
const atmosphere = useAtmosphere()
const {history} = useRouter()
Expand Down Expand Up @@ -131,9 +130,10 @@ const EmailPasswordAuthForm = forwardRef((props: Props, ref: any) => {
const domain = getSSODomainFromEmail(email)
if (domain && domain !== pendingDomain) {
setPendingDomain(domain)
// Fetch the url to verify SSO is configured for this domain.
// Don't cache it as we need a fresh one for login
const url = await getSSOUrl(atmosphere, email)
setSSODomain(domain)
setSSOURL(url || '')
setSSODomain(url ? domain : undefined)
}
}
}
Expand All @@ -148,8 +148,8 @@ const EmailPasswordAuthForm = forwardRef((props: Props, ref: any) => {

const tryLoginWithSSO = async (email: string) => {
const domain = getSSODomainFromEmail(email)!
const validSSOURL = domain === ssoDomain && ssoURL
const isProbablySSO = isSSO || !fields.password.value || validSSOURL
const hadValidSSOURL = domain === ssoDomain
const isProbablySSO = isSSO || !fields.password.value || hadValidSSOURL
const top = getOffsetTop?.() || 56
let optimisticPopup
if (isProbablySSO) {
Expand All @@ -168,7 +168,7 @@ const EmailPasswordAuthForm = forwardRef((props: Props, ref: any) => {
getOAuthPopupFeatures({width: 385, height: 576, top})
)
}
const url = validSSOURL || (await getSSOUrl(atmosphere, email))
const url = await getSSOUrl(atmosphere, email)
if (!url) {
optimisticPopup?.close()
return false
Expand Down
4 changes: 3 additions & 1 deletion packages/client/utils/getSAMLIdP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ const query = graphql`
`

const getSAMLIdP = async (atmosphere: Atmosphere, variables: getSAMLIdPQuery['variables']) => {
const res = await atmosphere.fetchQuery<getSAMLIdPQuery>(query, variables)
const res = await atmosphere.fetchQuery<getSAMLIdPQuery>(query, variables, {
fetchPolicy: 'network-only'
})
return res?.SAMLIdP ?? null
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const resolveDowngradeToStarter = async (
.execute(),
pg
.updateTable('SAML')
.set({metadata: null, url: null, lastUpdatedBy: user.id})
.set({metadata: null, metadataURL: null, lastUpdatedBy: user.id})
.where('orgId', '=', orgId)
.execute(),
updateTeamByOrgId(
Expand Down
3 changes: 2 additions & 1 deletion packages/server/graphql/private/mutations/loginSAML.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,14 @@ const loginSAML: MutationResolvers['loginSAML'] = async (
if (newMetadata) {
// The user is updating their SAML metadata
// Revalidate it & persist to DB
// Generate the URL to verify the metadata, don't persist it as it needs to be generated fresh
const url = getSignOnURL(metadata, normalizedName)
if (url instanceof Error) {
return standardError(url)
}
await pg
.updateTable('SAML')
.set({metadata: newMetadata, metadataURL: newMetadataURL, url})
.set({metadata: newMetadata, metadataURL: newMetadataURL})
.where('id', '=', normalizedName)
.execute()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type {Kysely} from 'kysely'

export async function up(db: Kysely<any>): Promise<void> {
await db.schema.alterTable('SAML').dropColumn('url').execute()
}

export async function down(db: Kysely<any>): Promise<void> {
await db.schema.alterTable('SAML').addColumn('url', 'varchar(2056)').execute()
}
20 changes: 13 additions & 7 deletions packages/server/utils/getSAMLURLFromEmail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import base64url from 'base64url'
import getSSODomainFromEmail from 'parabol-client/utils/getSSODomainFromEmail'
import {URL} from 'url'
import {DataLoaderWorker} from '../graphql/graphql'
import getSignOnURL from '../graphql/public/mutations/helpers/SAMLHelpers/getSignOnURL'
import getKysely from '../postgres/getKysely'

export const isSingleTenantSSO =
Expand All @@ -26,14 +27,17 @@ const getSAMLURLFromEmail = async (
if (isSingleTenantSSO) {
// For PPMI use
const pg = getKysely()
const instanceURLres = await pg
const instanceRes = await pg
.selectFrom('SAML')
.select('url')
.where('url', 'is not', null)
.select(['id', 'metadata'])
.where('metadata', 'is not', null)
.limit(1)
.executeTakeFirst()
const instanceURL = instanceURLres?.url
if (!instanceURL) return null
if (!instanceRes) return null
const {id, metadata} = instanceRes
if (!metadata) return null
const instanceURL = getSignOnURL(metadata, id)
if (instanceURL instanceof Error) return null
return urlWithRelayState(instanceURL, isInvited)
}
if (!email) return null
Expand All @@ -42,8 +46,10 @@ const getSAMLURLFromEmail = async (

const saml = await dataLoader.get('samlByDomain').load(domainName)
if (!saml) return null
const {url} = saml
if (!url) return null
const {id, metadata} = saml
if (!metadata) return null
const url = getSignOnURL(metadata, id)
if (url instanceof Error) return null
return urlWithRelayState(url, isInvited)
}

Expand Down

0 comments on commit 22d89e5

Please sign in to comment.