diff --git a/.circleci/config.yml b/.circleci/config.yml
index ebcf86415..57fdc36fd 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -372,11 +372,6 @@ workflows:
- get-version
- publish-approval
- - update-helm-documentation:
- filters: *filters-publish
- requires:
- - publish-helm-chart
-
- publish-npm:
filters:
tags:
@@ -1110,21 +1105,6 @@ jobs:
name: Publish Helm Chart
command: ./.circleci/publish_helm_chart.sh
- update-helm-documentation:
- <<: *docker-node-image
- working_directory: *work-dir
- steps:
- - checkout
- - attach_workspace:
- at: /tmp/ci/workspace
- - run: cat workspace/env-vars >> $BASH_ENV
- - add_ssh_keys:
- fingerprints:
- - '4d:68:70:66:49:97:ba:8b:8c:55:96:df:3d:be:6e:05'
- - run:
- name: Update Helm Documentation
- command: ./.circleci/update_helm_documentation.sh
-
publish-viewer-sandbox-cloudflare-pages:
<<: *docker-node-image
working_directory: *work-dir
diff --git a/.circleci/update_helm_documentation.sh b/.circleci/update_helm_documentation.sh
deleted file mode 100755
index dcd971ffd..000000000
--- a/.circleci/update_helm_documentation.sh
+++ /dev/null
@@ -1,71 +0,0 @@
-#!/usr/bin/env bash
-set -euo pipefail
-if ! command -v node &> /dev/null
-then
- echo "๐ node could not be found. Please install node (and ensure it is in your PATH) before trying again."
- exit 1
-fi
-
-if ! command -v git &> /dev/null
-then
- echo "๐ git could not be found. Please install git (and ensure it is in your PATH) before trying again."
- exit 1
-fi
-
-GIT_ROOT="$(git rev-parse --show-toplevel)"
-
-README_GENERATOR_DIR="${GIT_ROOT}/../readme-generator-for-helm"
-HELM_DIR="${GIT_ROOT}/../speckle-helm"
-HELM_GIT_TARGET_BRANCH="gh-pages"
-
-if [ ! -d "${README_GENERATOR_DIR}" ]; then
- echo "๐ญ Could not find 'readme-generator-for-helm' in a sibling directory"
- echo "๐ฉโ๐ฉโ๐งโ๐ง Proceeding with cloning readme-generator-for-helm to a sibling directory, 'readme-generator-for-helm'"
- git clone git@github.com:bitnami-labs/readme-generator-for-helm.git "${README_GENERATOR_DIR}"
-fi
-
-if [ -n "${CI}" ]; then
- git config --global user.email "devops+circleci@speckle.systems"
- git config --global user.name "CI"
-fi
-
-pushd "${README_GENERATOR_DIR}"
- echo "โจ Updating to the latest version of readme-generator-for-helm"
- git switch main
- git pull origin main
- npm install
-popd
-
-if [ ! -d "${HELM_DIR}" ]; then
- echo "๐ญ Could not find Speckle Helm in a sibling directory (named 'speckle-helm')"
- echo "๐ฉโ๐ฉโ๐งโ๐ง Proceeding with cloning Speckle's helm repository to a sibling directory, 'speckle-helm'"
- git clone git@github.com:specklesystems/helm.git "${HELM_DIR}"
-fi
-
-pushd "${HELM_DIR}"
- echo "โจ Updating to the latest version of Speckle helm"
- git switch main
- git pull origin main
- echo "๐ฝ Preparing gh-pages branch for updates"
- git switch "${HELM_GIT_TARGET_BRANCH}"
- git pull origin "${HELM_GIT_TARGET_BRANCH}"
-popd
-
-pushd "${GIT_ROOT}"
- echo "๐ Generating the documentation"
- node "${README_GENERATOR_DIR}/bin/index.js" \
- --config "${GIT_ROOT}/utils/helm/.helm-readme-configuration.json" \
- --values "${GIT_ROOT}/utils/helm/speckle-server/values.yaml" \
- --readme "${HELM_DIR}/README.md"
-popd
-
-pushd "${HELM_DIR}"
- echo "๐ณ Preparing commit to branch '${HELM_GIT_TARGET_BRANCH}' for Helm README..."
- if [[ $(git status --porcelain) ]]; then
- git add README.md
- git commit -m "Updating README with revised parameters from values.yaml of Helm Chart."
- git push --set-upstream origin "${HELM_GIT_TARGET_BRANCH}"
- fi
-popd
-
-echo "โ
All done ๐"
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 96c42f5c0..cfa71703e 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -36,14 +36,14 @@ repos:
args:
- --ignore=E501 # ignoring error about lines that are too long
- # - repo: local
- # hooks:
- # - id: helm-documentation
- # name: Helm Json Schema
- # language: system
- # files: utils\/helm\/speckle\-server\/values\.yaml
- # entry: utils/helm/update-schema-json.sh
- # description: If this fails it is because the values.yaml file was updated. Or has missing or incorrect documentation.
+ - repo: local
+ hooks:
+ - id: helm-documentation
+ name: Helm Json Schema
+ language: system
+ files: utils\/helm\/speckle\-server\/values\.yaml
+ entry: utils/helm/update-schema-json.sh
+ description: If this fails it is because the values.yaml file was updated. Or has missing or incorrect documentation.
# helmlint should occur after the json schema is updated
- repo: https://github.com/gruntwork-io/pre-commit
diff --git a/packages/frontend-2/components/viewer/PreSetupWrapper.vue b/packages/frontend-2/components/viewer/PreSetupWrapper.vue
index 9620a4c58..74217a9b6 100644
--- a/packages/frontend-2/components/viewer/PreSetupWrapper.vue
+++ b/packages/frontend-2/components/viewer/PreSetupWrapper.vue
@@ -126,6 +126,7 @@ import { useFilterUtilities } from '~/lib/viewer/composables/ui'
import { projectsRoute, workspaceRoute } from '~~/lib/common/helpers/route'
import { useMixpanel } from '~/lib/core/composables/mp'
import { writableAsyncComputed } from '~/lib/common/composables/async'
+import { parseUrlParameters, resourceBuilder } from '@speckle/shared/viewer/route'
graphql(`
fragment ModelPageProject on Project {
@@ -197,15 +198,20 @@ const limitsDialogType = ref<'version' | 'comment' | 'federated'>('version')
// Check for missing referencedObject in url referenced versions (out of plan limits)
const hasMissingReferencedObject = computed(() => {
- const resourceIds = resourceIdString.value.split(',')
+ const resources = parseUrlParameters(resourceIdString.value)
const result = modelsAndVersionIds.value.some((item) => {
- const version = item.model?.versions?.items?.find((v) => v.id === item.versionId)
+ const version = item.model?.loadedVersion?.items?.find(
+ (v) => v.id === item.versionId
+ )
- if (version && version.referencedObject === null) {
- // Check if this model+version is in the URL (latest version always available)
- const modelVersionString = `${item.model.id}@${item.versionId}`.toLowerCase()
- const isInUrl = resourceIds.some((r) => r.toLowerCase() === modelVersionString)
+ if (!version || version.referencedObject === null) {
+ const modelVersionString = resourceBuilder()
+ .addModel(item.model.id, item.versionId)
+ .toString()
+ const isInUrl = resources.some(
+ (r) => r.toString().toLowerCase() === modelVersionString
+ )
return isInUrl
}
@@ -298,18 +304,11 @@ watch(
}
showLimitsDialog.value = true
return
- }
-
- // If no workspace and no missing objects, don't show dialog
- if (!project.value?.workspace) {
- showLimitsDialog.value = false
- return
- }
-
- // Only show comment dialog if it's a federated view AND we have a missing referenced object
- if (missingThread && isFederated.value && hasMissingReferencedObject.value) {
+ } else if (missingThread && isFederated.value && hasMissingReferencedObject.value) {
limitsDialogType.value = 'comment'
showLimitsDialog.value = true
+ } else {
+ showLimitsDialog.value = false
}
},
{ immediate: true }
diff --git a/packages/frontend-2/pages/book-a-demo.vue b/packages/frontend-2/pages/book-a-demo.vue
index 5280b8964..92b81da2d 100644
--- a/packages/frontend-2/pages/book-a-demo.vue
+++ b/packages/frontend-2/pages/book-a-demo.vue
@@ -20,11 +20,13 @@
- Book an intro call
+ Do you want a personal onboarding call?
Find a time
- We'd love to help you get started
+
+ Get started like our most successful users
+
@@ -77,13 +79,12 @@ const showEmbed = ref(false)
const options = computed(() => [
{
value: 'yes',
- title: `Yes, let's talk`,
- subtitle: 'Find a time in the next step'
+ title: `Yes, help me get started`,
+ subtitle: 'Schedule your 15 minute call in the next step'
},
{
value: 'no',
- title: 'No, maybe later',
- subtitle: 'You can also book a time later'
+ title: 'No, I will pass for now'
}
])
diff --git a/packages/server/modules/auth/domain/const.ts b/packages/server/modules/auth/domain/const.ts
new file mode 100644
index 000000000..51c604617
--- /dev/null
+++ b/packages/server/modules/auth/domain/const.ts
@@ -0,0 +1,9 @@
+export const ExpectedAuthFailure = {
+ UnverifiedEmailSSOLoginError: 'UnverifiedEmailSSOLoginError',
+ UserInputError: 'UserInputError',
+ InviteNotFoundError: 'InviteNotFoundError',
+ InvalidGrantError: 'InvalidGrantError'
+} as const
+
+export type ExpectedAuthFailure =
+ (typeof ExpectedAuthFailure)[keyof typeof ExpectedAuthFailure]
diff --git a/packages/server/modules/auth/index.ts b/packages/server/modules/auth/index.ts
index 68b81408c..cd39db4dc 100644
--- a/packages/server/modules/auth/index.ts
+++ b/packages/server/modules/auth/index.ts
@@ -120,7 +120,9 @@ const commonBuilderDeps = {
validateServerInvite,
finalizeInvitedServerRegistration,
resolveAuthRedirectPath,
- passportAuthenticateHandlerBuilder: passportAuthenticateHandlerBuilderFactory()
+ passportAuthenticateHandlerBuilder: passportAuthenticateHandlerBuilderFactory({
+ resolveAuthRedirectPath
+ })
}
const setupStrategies = setupStrategiesFactory({
githubStrategyBuilder: githubStrategyBuilderFactory({
diff --git a/packages/server/modules/auth/rest/index.ts b/packages/server/modules/auth/rest/index.ts
index a46cf4033..c183f95b2 100644
--- a/packages/server/modules/auth/rest/index.ts
+++ b/packages/server/modules/auth/rest/index.ts
@@ -113,6 +113,15 @@ export default function (app: Express) {
app.options('/auth/token', corsMiddlewareFactory())
app.post('/auth/token', corsMiddlewareFactory(), async (req, res) => {
try {
+ if (!req.body.appId)
+ throw new BadRequestError(
+ `Invalid request, insufficient information provided. App Id is required.`
+ )
+ if (!req.body.appSecret)
+ throw new BadRequestError(
+ `Invalid request, insufficient information provided. App Secret is required.`
+ )
+
const createRefreshToken = createRefreshTokenFactory({ db })
const getApp = getAppFactory({ db })
const createAppToken = createAppTokenFactory({
@@ -143,9 +152,11 @@ export default function (app: Express) {
})
// Token refresh
- if (req.body.refreshToken) {
- if (!req.body.appId || !req.body.appSecret)
- throw new BadRequestError('Invalid request - App Id and Secret are required.')
+ if ('refreshToken' in req.body) {
+ if (!req.body.refreshToken)
+ throw new BadRequestError(
+ 'Invalid request, insufficient information provided. A valid refresh token is required.'
+ )
const authResponse = await withOperationLogging(
async () =>
@@ -164,14 +175,13 @@ export default function (app: Express) {
}
// Access-code - token exchange
- if (
- !req.body.appId ||
- !req.body.appSecret ||
- !req.body.accessCode ||
- !req.body.challenge
- )
+ if (!req.body.accessCode)
throw new BadRequestError(
- `Invalid request, insufficient information provided in the request. App Id, Secret, Access Code, and Challenge are required.`
+ `Invalid request, insufficient information provided. Access Code is required.`
+ )
+ if (!req.body.challenge)
+ throw new BadRequestError(
+ `Invalid request, insufficient information provided. Challenge is required.`
)
const authResponse = await withOperationLogging(
diff --git a/packages/server/modules/auth/services/passportService.ts b/packages/server/modules/auth/services/passportService.ts
index 2b2ffe618..499b8d8ef 100644
--- a/packages/server/modules/auth/services/passportService.ts
+++ b/packages/server/modules/auth/services/passportService.ts
@@ -1,14 +1,11 @@
-import passport, { Strategy, AuthenticateOptions } from 'passport'
-import { getFrontendOrigin } from '@/modules/shared/helpers/envHelper'
-import {
- UnverifiedEmailSSOLoginError,
- UserInputError
-} from '@/modules/core/errors/userinput'
+import type { Strategy, AuthenticateOptions } from 'passport'
+import passport from 'passport'
import type { Request, Response, NextFunction, RequestHandler } from 'express'
-import { ensureError, Optional } from '@speckle/shared'
+import { ensureError, type Optional, throwUncoveredError } from '@speckle/shared'
import { get, isArray, isObjectLike, isString } from 'lodash'
-import { PassportAuthenticateHandlerBuilder } from '@/modules/auth/domain/operations'
-import { InviteNotFoundError } from '@/modules/serverinvites/errors'
+import type { PassportAuthenticateHandlerBuilder } from '@/modules/auth/domain/operations'
+import { ExpectedAuthFailure } from '@/modules/auth/domain/const'
+import type { ResolveAuthRedirectPath } from '@/modules/serverinvites/services/operations'
const resolveInfoMessage = (
info?: Optional | Array>
@@ -27,61 +24,147 @@ const resolveInfoMessage = (
return null
}
+const resolveFailureType = (
+ info?: Optional | Array>
+) => {
+ if (!info) return null
+ if (isString(info)) return null
+ if (isArray(info)) return null
+ if (isObjectLike(info)) {
+ const failureType = get(info, 'failureType')
+ if (
+ isString(failureType) &&
+ Object.values(ExpectedAuthFailure).includes(failureType as ExpectedAuthFailure)
+ ) {
+ return failureType as ExpectedAuthFailure
+ }
+ }
+ return null
+}
+
+const resolveEmail = (
+ info?: Optional | Array>
+) => {
+ if (!info) return ''
+ if (isString(info)) return ''
+ if (isArray(info)) return ''
+ if (isObjectLike(info)) {
+ const email = get(info, 'email')
+ if (isString(email)) {
+ return email
+ }
+ }
+ return ''
+}
+
const defaultErrorPath = (message: string) => `/error?message=${message}`
const unverifiedEmailPath = (email: string) => `/error-email-verify?email=${email}`
+const buildRedirectUrl = (params: {
+ resolveAuthRedirectPath: ResolveAuthRedirectPath
+ path: string
+}) => new URL(params.path, params.resolveAuthRedirectPath()).toString()
+
export const passportAuthenticationCallbackFactory =
(context: {
strategy: Strategy | string
req: Request
res: Response
next: NextFunction
+ resolveAuthRedirectPath: ResolveAuthRedirectPath
}) =>
(
- e: unknown,
+ callbackError: unknown,
user: Optional,
info: Optional | Array>
) => {
- const { strategy, req, res, next } = context
+ const { strategy, req, res, next, resolveAuthRedirectPath } = context
- if (user && !e) {
+ let e = callbackError
+ let failureType = resolveFailureType(info)
+
+ // WORKAROUND
+ // passportjs states that 'verify' method of the strategy should not pass in
+ // an error for user input problems.
+ // Unfortunately openid-client <6.0.0 does not provide a third 'info' parameter
+ // so we rely on user-input problems being passed to callback as errors.
+ // This is a workaround until we upgrade to openid-client >=6.0.0
+ if (e && strategy === 'oidc' && failureType === null) {
+ switch (e.constructor.name) {
+ case ExpectedAuthFailure.UserInputError:
+ case ExpectedAuthFailure.InviteNotFoundError:
+ case ExpectedAuthFailure.UnverifiedEmailSSOLoginError:
+ // the error was being overloaded with user input problem information
+ // so we need to extract it and set it as the info
+ // and set the error to null
+ failureType = e.constructor.name
+ e = null
+ break
+ default:
+ // what we have is an unexpected error, so nothing needs to change
+ break
+ }
+ }
+
+ if (e) {
+ const err = ensureError(
+ e,
+ 'Unknown authentication error. Please contact server admins'
+ )
+
+ // unknown and unexpected error
+ req.log.error({ err, strategy }, 'Authentication error for strategy "{strategy}"')
+ return next(err)
+ }
+
+ if (user && failureType === null) {
req.user = user
// user authenticated successfully
next()
return
}
+ // no user, but no error either. This is expected in some cases (e.g. user input error)
+ // in this case, we need to redirect the user to the correct page
const infoMsg = resolveInfoMessage(info)
- if (!user && !e) {
- // no user despite there being no error, so authentication failed
- const message = infoMsg || 'Failed to authenticate, contact server admins'
- res.redirect(new URL(defaultErrorPath(message), getFrontendOrigin()).toString())
- return
- }
-
- const err = ensureError(
- e,
- 'Unknown authentication error. Please contact server admins'
- )
- switch (err.constructor) {
- case UserInputError:
- case InviteNotFoundError:
- const message = infoMsg || err.message
- res.redirect(new URL(defaultErrorPath(message), getFrontendOrigin()).toString())
- return
- case UnverifiedEmailSSOLoginError:
- const email = (err as UnverifiedEmailSSOLoginError).info()?.email || ''
+ switch (failureType) {
+ case ExpectedAuthFailure.UserInputError:
+ case ExpectedAuthFailure.InviteNotFoundError:
+ case ExpectedAuthFailure.InvalidGrantError:
res.redirect(
- new URL(unverifiedEmailPath(email), getFrontendOrigin()).toString()
+ buildRedirectUrl({
+ resolveAuthRedirectPath,
+ path: defaultErrorPath(
+ infoMsg || 'Failed to authenticate, contact server admins'
+ )
+ })
+ )
+ return
+ case ExpectedAuthFailure.UnverifiedEmailSSOLoginError:
+ const email = resolveEmail(info)
+ res.redirect(
+ buildRedirectUrl({
+ resolveAuthRedirectPath,
+ path: unverifiedEmailPath(email)
+ })
+ )
+ return
+ case null:
+ // unexpected error or missing info
+ req.log.error(
+ { info, strategy },
+ "Authentication error for strategy '{strategy}' encountered an unexpected failure type or 'info' parameter is missing or invalid"
+ )
+ const message = infoMsg || 'Failed to authenticate, contact server admins'
+ res.redirect(
+ buildRedirectUrl({
+ resolveAuthRedirectPath,
+ path: defaultErrorPath(message)
+ })
)
return
default:
- req.log.error(
- { err, strategy },
- 'Authentication error for strategy "{strategy}"'
- )
- return next(err)
- // throwUncoveredError(err) //TODO at some point in the future, ideally change error handling to use this
+ throwUncoveredError(failureType)
}
}
@@ -90,7 +173,9 @@ export const passportAuthenticationCallbackFactory =
* (passport.authenticate() by default doesn't, so don't use it)
*/
export const passportAuthenticateHandlerBuilderFactory =
- (): PassportAuthenticateHandlerBuilder =>
+ (deps: {
+ resolveAuthRedirectPath: ResolveAuthRedirectPath
+ }): PassportAuthenticateHandlerBuilder =>
(
strategy: Strategy | string,
options: Optional = undefined
@@ -99,7 +184,7 @@ export const passportAuthenticateHandlerBuilderFactory =
passport.authenticate(
strategy,
options || {},
- passportAuthenticationCallbackFactory({ strategy, req, res, next })
+ passportAuthenticationCallbackFactory({ ...deps, strategy, req, res, next })
)(req, res, next)
}
}
diff --git a/packages/server/modules/auth/strategies/github.ts b/packages/server/modules/auth/strategies/github.ts
index f5b52a5f0..6c120425c 100644
--- a/packages/server/modules/auth/strategies/github.ts
+++ b/packages/server/modules/auth/strategies/github.ts
@@ -34,6 +34,7 @@ import crs from 'crypto-random-string'
import { GetServerInfo } from '@/modules/core/domain/server/operations'
import { EnvironmentResourceError } from '@/modules/shared/errors'
import { InviteNotFoundError } from '@/modules/serverinvites/errors'
+import { ExpectedAuthFailure } from '@/modules/auth/domain/const'
const githubStrategyBuilderFactory =
(deps: {
@@ -164,11 +165,14 @@ const githubStrategyBuilderFactory =
case InviteNotFoundError:
case UnverifiedEmailSSOLoginError:
logger.info({ err: e }, 'Auth error for GitHub strategy')
- // note; passportjs suggests that err should be null for user input errors.
- // However, we are relying on the error being passed to `passportAuthenticationCallbackFactory` and handling it there
- return done(e, false, { message: e.message })
+ // note; passportjs suggests err should be null for user input errors.
+ // We also need to pass the error type in the info parameter
+ // so `passportAuthenticationCallbackFactory` can handle redirects appropriately
+ return done(null, false, {
+ message: e.message,
+ failureType: e.constructor.name as ExpectedAuthFailure
+ })
default:
- logger.error({ err: e }, 'Auth error for GitHub strategy')
return done(e, false, { message: e.message })
}
}
diff --git a/packages/server/modules/auth/strategies/google.ts b/packages/server/modules/auth/strategies/google.ts
index 52255a5aa..69cd5dfcd 100644
--- a/packages/server/modules/auth/strategies/google.ts
+++ b/packages/server/modules/auth/strategies/google.ts
@@ -28,7 +28,7 @@ import {
} from '@/modules/core/domain/users/operations'
import { GetServerInfo } from '@/modules/core/domain/server/operations'
import { EnvironmentResourceError } from '@/modules/shared/errors'
-import { InviteNotFoundError } from '@/modules/serverinvites/errors'
+import { ExpectedAuthFailure } from '@/modules/auth/domain/const'
const googleStrategyBuilderFactory =
(deps: {
@@ -142,15 +142,47 @@ const googleStrategyBuilderFactory =
err,
'Unexpected issue occured while authenticating with Google'
)
- switch (e.constructor) {
- case UnverifiedEmailSSOLoginError:
- case UserInputError:
- case InviteNotFoundError:
+ switch (e.constructor.name) {
+ case ExpectedAuthFailure.UserInputError:
+ case ExpectedAuthFailure.InviteNotFoundError:
logger.info({ err: e }, 'Auth error for Google strategy')
- // note; passportjs suggests that err should be null for user input errors.
- // However, we are relying on the error being passed to `passportAuthenticationCallbackFactory` and handling it there
- return done(e, false, { message: e.message })
+ // note; passportjs suggests err should be null for user input errors.
+ // We also need to pass the error type in the info parameter
+ // so `passportAuthenticationCallbackFactory` can handle redirects appropriately
+ return done(null, false, {
+ message: e.message,
+ failureType: e.constructor.name as ExpectedAuthFailure
+ })
+ case ExpectedAuthFailure.UnverifiedEmailSSOLoginError:
+ logger.info({ err: e }, 'Auth error for Google strategy')
+ // note; passportjs suggests err should be null for user input errors.
+ // We also need to pass the error type in the info parameter
+ // so `passportAuthenticationCallbackFactory` can handle redirects appropriately
+ return done(null, false, {
+ message: e.message,
+ failureType: e.constructor.name as ExpectedAuthFailure,
+ email: (e as UnverifiedEmailSSOLoginError).info().email
+ })
default:
+ // handle other common errors thrown by the underlying client libraries
+ if (
+ e.name === 'TokenError' &&
+ 'code' in e &&
+ e.code === 'invalid_grant'
+ ) {
+ req.log.warn(
+ { err: e },
+ "Authentication error for strategy 'google' encountered an Invalid Grant error"
+ )
+ // This is a common error from Google and a number of reasons
+ // can cause it. Many user-related issues, so we will treat it as user-related.
+ // https://blog.timekit.io/google-oauth-invalid-grant-nightmare-and-how-to-fix-it-9f4efaf1da35
+ return done(null, false, {
+ message: e.message,
+ failureType: ExpectedAuthFailure.InvalidGrantError
+ })
+ }
+
logger.error({ err: e }, 'Auth error for Google strategy')
return done(e, false, { message: e.message })
}
diff --git a/packages/server/modules/auth/strategies/oidc.ts b/packages/server/modules/auth/strategies/oidc.ts
index 1271ea0c4..19898f27b 100644
--- a/packages/server/modules/auth/strategies/oidc.ts
+++ b/packages/server/modules/auth/strategies/oidc.ts
@@ -164,6 +164,7 @@ const oidcStrategyBuilderFactory =
logger.info({ err: e }, 'Auth error for OIDC strategy')
// note; passportjs suggests that err should be null for user input errors.
// However, we are relying on the error being passed to `passportAuthenticationCallbackFactory` and handling it there
+ // as openid-client <6.0.0 does not support a third 'info' parameter for the callback function.
return done(e, undefined)
default:
logger.error({ err: e }, 'Auth error for OIDC strategy')
diff --git a/packages/server/modules/auth/tests/auth.spec.ts b/packages/server/modules/auth/tests/auth.spec.ts
index 231a9fad1..996480ad2 100644
--- a/packages/server/modules/auth/tests/auth.spec.ts
+++ b/packages/server/modules/auth/tests/auth.spec.ts
@@ -74,6 +74,7 @@ import { authorizeResolver } from '@/modules/shared'
import { UserInputError } from '@/modules/core/errors/userinput'
import { createRandomEmail } from '@/modules/core/helpers/testHelpers'
import cryptoRandomString from 'crypto-random-string'
+import { getFrontendOrigin } from '@/modules/shared/helpers/envHelper'
const getServerInfo = getServerInfoFactory({ db })
const getUser = getUserFactory({ db })
@@ -691,7 +692,8 @@ describe('Auth @auth', () => {
strategy: 'wotStrategy',
req,
res,
- next
+ next,
+ resolveAuthRedirectPath: () => getFrontendOrigin()
})
const userId = cryptoRandomString({ length: 4 })
@@ -709,7 +711,7 @@ describe('Auth @auth', () => {
'next request handler should have been called'
).to.equal(1)
})
- it('Should handle case where there is an unexpected error and a user', async () => {
+ it('Should handle case where there is an unexpected error but also a user', async () => {
const req = httpMocks.createRequest()
req.log = logger
const res = httpMocks.createResponse()
@@ -725,7 +727,8 @@ describe('Auth @auth', () => {
strategy: 'wotStrategy',
req,
res,
- next
+ next,
+ resolveAuthRedirectPath: () => getFrontendOrigin()
})
SUT(
@@ -745,7 +748,7 @@ describe('Auth @auth', () => {
'next request handler should have been called'
).to.equal(1)
})
- it('Should handle case where there is a user-derived error and a user', async () => {
+ it('Should handle case where there is a user-derived failure but also a user', async () => {
const req = httpMocks.createRequest()
req.log = logger
const res = httpMocks.createResponse()
@@ -761,13 +764,14 @@ describe('Auth @auth', () => {
strategy: 'wotStrategy',
req,
res,
- next
+ next,
+ resolveAuthRedirectPath: () => getFrontendOrigin()
})
SUT(
- new UserInputError('I brrrrroke'),
+ null,
{ id: cryptoRandomString({ length: 4 }), email: createRandomEmail() },
- undefined
+ { failureType: 'UserInputError' }
)
expect(
res._getRedirectUrl().includes('/error'),
@@ -799,7 +803,8 @@ describe('Auth @auth', () => {
strategy: 'wotStrategy',
req,
res,
- next
+ next,
+ resolveAuthRedirectPath: () => getFrontendOrigin()
})
SUT(null, undefined, undefined)
@@ -817,7 +822,7 @@ describe('Auth @auth', () => {
'next request handler should not have been called'
).to.equal(0)
})
- it('Should handle case where there is a user-derived error but no user', async () => {
+ it('Should handle case where there is a user-derived failure and no user', async () => {
const req = httpMocks.createRequest()
req.log = logger
const res = httpMocks.createResponse()
@@ -833,10 +838,11 @@ describe('Auth @auth', () => {
strategy: 'wotStrategy',
req,
res,
- next
+ next,
+ resolveAuthRedirectPath: () => getFrontendOrigin()
})
- SUT(new UserInputError('I brrrrroke'), undefined, undefined)
+ SUT(null, undefined, { failureType: 'UserInputError' })
expect(
res._getRedirectUrl().includes('/error'),
`Redirect url was '${res._getRedirectUrl()}'`
@@ -867,7 +873,8 @@ describe('Auth @auth', () => {
strategy: 'wotStrategy',
req,
res,
- next
+ next,
+ resolveAuthRedirectPath: () => getFrontendOrigin()
})
SUT(new Error('surprise!!!'), undefined, undefined)
@@ -881,5 +888,79 @@ describe('Auth @auth', () => {
'next request handler should have been called with an error "next(err)"'
).to.equal(1)
})
+ it('should handle invalid grant user-derived failure and no user', async () => {
+ const req = httpMocks.createRequest()
+ req.log = logger
+ const res = httpMocks.createResponse()
+ let errorCalledCounter = 0
+ let nextCalledCounter = 0
+ const next = (err: unknown) => {
+ if (err) {
+ errorCalledCounter++
+ }
+ nextCalledCounter++
+ }
+ const SUT = passportAuthenticationCallbackFactory({
+ strategy: 'google',
+ req,
+ res,
+ next,
+ resolveAuthRedirectPath: () => getFrontendOrigin()
+ })
+
+ SUT(null, undefined, {
+ failureType: 'InvalidGrantError',
+ message: 'Some kind of invalid grant'
+ })
+ expect(
+ res._getRedirectUrl().includes('/error'),
+ `Redirect url was '${res._getRedirectUrl()}'`
+ ).to.be.true
+ expect(req).not.to.have.property('user')
+ expect(
+ errorCalledCounter,
+ 'error request handler "next(err)" should not have been called'
+ ).to.equal(0)
+ expect(
+ nextCalledCounter,
+ 'next request handler should not have been called'
+ ).to.equal(0)
+ })
+ //TODO remove this if we upgrade to openid-client >=6.0.0
+ it('should handle case for OIDC with a user-derived error and no user', async () => {
+ const req = httpMocks.createRequest()
+ req.log = logger
+ const res = httpMocks.createResponse()
+ let errorCalledCounter = 0
+ let nextCalledCounter = 0
+ const next = (err: unknown) => {
+ if (err) {
+ errorCalledCounter++
+ }
+ nextCalledCounter++
+ }
+ const SUT = passportAuthenticationCallbackFactory({
+ strategy: 'oidc',
+ req,
+ res,
+ next,
+ resolveAuthRedirectPath: () => getFrontendOrigin()
+ })
+
+ SUT(new UserInputError('oidc version <6 is special'), undefined, undefined)
+ expect(
+ res._getRedirectUrl().includes('/error'),
+ `Redirect url was '${res._getRedirectUrl()}'`
+ ).to.be.true
+ expect(req).not.to.have.property('user')
+ expect(
+ errorCalledCounter,
+ 'error request handler "next(err)" should not have been called'
+ ).to.equal(0)
+ expect(
+ nextCalledCounter,
+ 'next request handler should not have been called'
+ ).to.equal(0)
+ })
})
})
diff --git a/packages/server/modules/core/graph/resolvers/users.ts b/packages/server/modules/core/graph/resolvers/users.ts
index 092f963f3..b5edeafc3 100644
--- a/packages/server/modules/core/graph/resolvers/users.ts
+++ b/packages/server/modules/core/graph/resolvers/users.ts
@@ -131,7 +131,7 @@ export = {
await validateScopes(context.scopes, Scopes.Users.Read)
if (args.query.length < 3)
- throw new BadRequestError('Search query must be at least 3 carachters.')
+ throw new BadRequestError('Search query must be at least 3 characters.')
if (args.limit && args.limit > 100)
throw new BadRequestError(
diff --git a/packages/server/modules/core/services/objects/management.ts b/packages/server/modules/core/services/objects/management.ts
index c8c3a0039..fe665ba02 100644
--- a/packages/server/modules/core/services/objects/management.ts
+++ b/packages/server/modules/core/services/objects/management.ts
@@ -34,6 +34,12 @@ const prepInsertionObject = (
obj.id =
obj.id || crypto.createHash('md5').update(JSON.stringify(obj)).digest('hex') // generate a hash if none is present
+ if (obj.id.length !== 32) {
+ throw new ObjectHandlingError(
+ `Invalid object ID. Object ID: ${obj.id}. Object ID's must be hashes represented by a string of 32 characters.`
+ )
+ }
+
const stringifiedObj = JSON.stringify(obj)
const objectByteSize = estimateStringMegabyteSize(stringifiedObj)
if (objectByteSize > MAX_OBJECT_SIZE_MB) {
diff --git a/utils/helm/update-schema-json.sh b/utils/helm/update-schema-json.sh
index d4078a34e..6d1d5813f 100755
--- a/utils/helm/update-schema-json.sh
+++ b/utils/helm/update-schema-json.sh
@@ -21,7 +21,14 @@ JSON_SCHEMA_PATH="${GIT_ROOT}/utils/helm/speckle-server/values.schema.json"
if [ ! -d "${README_GENERATOR_DIR}" ]; then
echo "๐ญ Could not find readme-generator-for-helm in a sibling directory to speckle-server"
echo "๐ฉโ๐ฉโ๐งโ๐ง Proceeding with cloning readme-generator-for-helm to a sibling directory, readme-generator-for-helm"
- git clone git@github.com:bitnami-labs/readme-generator-for-helm.git "${README_GENERATOR_DIR}"
+ SSH_OUTPUT="$(ssh -T git@github.com 2>&1 || true)"
+ if echo "${SSH_OUTPUT}" | grep -q 'successfully authenticated'; then
+ echo "๐ SSH authentication successful, cloning using SSH"
+ git clone git@github.com:bitnami-labs/readme-generator-for-helm.git "${README_GENERATOR_DIR}"
+ else
+ echo "๐ SSH authentication failed, cloning using HTTPS"
+ git clone https://github.com/bitnami-labs/readme-generator-for-helm "${README_GENERATOR_DIR}"
+ fi
fi
pushd "${README_GENERATOR_DIR}"