Skip to content

Commit

Permalink
Merge pull request #2708 from Infisical/misc/oidc-setup-extra-handling
Browse files Browse the repository at this point in the history
misc: added OIDC error and edge-case handling
  • Loading branch information
maidul98 authored Nov 18, 2024
2 parents 6603364 + 089a7e8 commit 6ff1602
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 15 deletions.
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 };
};

0 comments on commit 6ff1602

Please sign in to comment.