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.

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

misc: added OIDC error and edge-case handling #2708

Merged
merged 4 commits into from
Nov 18, 2024
Merged
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
3 changes: 3 additions & 0 deletions backend/e2e-test/mocks/smtp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ export const mockSmtpServer = (): TSmtpService => {
return {
sendMail: async (data) => {
storage.push(data);
},
verify: async () => {
return true;
}
};
};
56 changes: 46 additions & 10 deletions backend/src/ee/services/oidc/oidc-config-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
infisicalSymmetricDecrypt,
infisicalSymmetricEncypt
} from "@app/lib/crypto/encryption";
import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors";
import { BadRequestError, ForbiddenRequestError, NotFoundError, OidcAuthError } from "@app/lib/errors";
import { AuthMethod, AuthTokenType } from "@app/services/auth/auth-type";
import { TAuthTokenServiceFactory } from "@app/services/auth-token/auth-token-service";
import { TokenType } from "@app/services/auth-token/auth-token-types";
Expand Down Expand Up @@ -56,7 +56,7 @@ type TOidcConfigServiceFactoryDep = {
orgBotDAL: Pick<TOrgBotDALFactory, "findOne" | "create" | "transaction">;
licenseService: Pick<TLicenseServiceFactory, "getPlan" | "updateSubscriptionOrgMemberCount">;
tokenService: Pick<TAuthTokenServiceFactory, "createTokenForUser">;
smtpService: Pick<TSmtpService, "sendMail">;
smtpService: Pick<TSmtpService, "sendMail" | "verify">;
permissionService: Pick<TPermissionServiceFactory, "getOrgPermission">;
oidcConfigDAL: Pick<TOidcConfigDALFactory, "findOne" | "update" | "create">;
};
Expand Down Expand Up @@ -223,13 +223,31 @@ export const oidcConfigServiceFactory = ({
let newUser: TUsers | undefined;

if (serverCfg.trustOidcEmails) {
// we prioritize getting the most complete user to create the new alias under
newUser = await userDAL.findOne(
{
email,
isEmailVerified: true
},
tx
);

if (!newUser) {
// this fetches user entries created via invites
newUser = await userDAL.findOne(
{
username: email
},
tx
);

if (newUser && !newUser.isEmailVerified) {
// we automatically mark it as email-verified because we've configured trust for OIDC emails
newUser = await userDAL.updateById(newUser.id, {
isEmailVerified: true
});
}
}
}

if (!newUser) {
Expand Down Expand Up @@ -332,14 +350,20 @@ export const oidcConfigServiceFactory = ({
userId: user.id
});

await smtpService.sendMail({
template: SmtpTemplates.EmailVerification,
subjectLine: "Infisical confirmation code",
recipients: [user.email],
substitutions: {
code: token
}
});
await smtpService
.sendMail({
template: SmtpTemplates.EmailVerification,
subjectLine: "Infisical confirmation code",
recipients: [user.email],
substitutions: {
code: token
}
})
.catch((err: Error) => {
throw new OidcAuthError({
message: `Error sending email confirmation code for user registration - contact the Infisical instance admin. ${err.message}`
});
});
}

return { isUserCompleted, providerAuthToken };
Expand Down Expand Up @@ -395,6 +419,18 @@ export const oidcConfigServiceFactory = ({
message: `Organization bot for organization with ID '${org.id}' not found`,
name: "OrgBotNotFound"
});

const serverCfg = await getServerCfg();
if (isActive && !serverCfg.trustOidcEmails) {
const isSmtpConnected = await smtpService.verify();
if (!isSmtpConnected) {
throw new BadRequestError({
message:
"Cannot enable OIDC when there are issues with the instance's SMTP configuration. Bypass this by turning on trust for OIDC emails in the server admin console."
});
}
}

const key = infisicalSymmetricDecrypt({
ciphertext: orgBot.encryptedSymmetricKey,
iv: orgBot.symmetricKeyIV,
Expand Down
12 changes: 12 additions & 0 deletions backend/src/lib/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,15 @@ export class ScimRequestError extends Error {
this.status = status;
}
}

export class OidcAuthError extends Error {
name: string;

error: unknown;

constructor({ name, error, message }: { message?: string; name?: string; error?: unknown }) {
super(message || "Something went wrong");
this.name = name || "OidcAuthError";
this.error = error;
}
}
6 changes: 3 additions & 3 deletions backend/src/server/boot-strap-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ export const bootstrapCheck = async ({ db }: BootstrapOpt) => {
await createTransport(smtpCfg)
.verify()
.then(async () => {
console.info("SMTP successfully connected");
console.info(`SMTP - Verified connection to ${appCfg.SMTP_HOST}:${appCfg.SMTP_PORT}`);
})
.catch((err) => {
console.error(`SMTP - Failed to connect to ${appCfg.SMTP_HOST}:${appCfg.SMTP_PORT}`);
.catch((err: Error) => {
console.error(`SMTP - Failed to connect to ${appCfg.SMTP_HOST}:${appCfg.SMTP_PORT} - ${err.message}`);
logger.error(err);
});

Expand Down
6 changes: 5 additions & 1 deletion backend/src/server/plugins/error-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
GatewayTimeoutError,
InternalServerError,
NotFoundError,
OidcAuthError,
RateLimitError,
ScimRequestError,
UnauthorizedError
Expand Down Expand Up @@ -83,7 +84,10 @@ export const fastifyErrHandler = fastifyPlugin(async (server: FastifyZodProvider
status: error.status,
detail: error.detail
});
// Handle JWT errors and make them more human-readable for the end-user.
} else if (error instanceof OidcAuthError) {
void res
.status(HttpStatusCodes.InternalServerError)
.send({ statusCode: HttpStatusCodes.InternalServerError, message: error.message, error: error.name });
} else if (error instanceof jwt.JsonWebTokenError) {
const message = (() => {
if (error.message === JWTErrors.JwtExpired) {
Expand Down
18 changes: 17 additions & 1 deletion backend/src/services/smtp/smtp-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,21 @@ export const smtpServiceFactory = (cfg: TSmtpConfig) => {
}
};

return { sendMail };
const verify = async () => {
const isConnected = smtp
.verify()
.then(async () => {
logger.info("SMTP connected");
return true;
})
.catch((err: Error) => {
logger.error("SMTP error");
logger.error(err);
return false;
});

return isConnected;
};

return { sendMail, verify };
};
Loading