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

Local kratos email verification with mailslurper #1648

Merged
merged 11 commits into from
Dec 13, 2022
6 changes: 4 additions & 2 deletions packages/hash/api/src/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,10 @@ const setupAuth = (params: {
if (err.response && err.response.status === 403) {
/** @todo: figure out if this should be handled here, or in the next.js app (when implementing 2FA) */
}
logger.error(
`Could not fetch session, got error: [${err.response?.status}] ${err.response?.data}`,
logger.debug(
`Kratos response error: Could not fetch session, got: [${
err.response?.status
}] ${JSON.stringify(err.response?.data)}`,
);
return undefined;
});
Expand Down
1 change: 1 addition & 0 deletions packages/hash/external-services/kratos/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ ENV API_SECRET=$API_SECRET
RUN mkdir -p /etc/config/kratos && \
mkdir -p /home/ory/.postgresql

COPY ./templates /etc/config/kratos/templates
COPY ./hooks /etc/config/kratos/hooks
COPY ./identity.schema.json /etc/config/kratos/
COPY kratos.$ENV.yml /etc/config/kratos/kratos.$ENV.yml
Expand Down
24 changes: 24 additions & 0 deletions packages/hash/external-services/kratos/kratos.dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ selfservice:
password:
enabled: true

link:
config:
# The URL for verification emails are set through the link method
# but we're using the code method, so we disable this method for usage.
enabled: false
# Set through SELFSERVICE_METHODS_LINK_CONFIG_BASE_URL
base_url: http://localhost:3000/api/ory
code:
config:
# and make sure to enable the code method.
enabled: true
Comment on lines +27 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, should we make an effort from here on to start copying in the documentation from the ory kratos website?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's very helpful for values that we know we will inject new values into such as this and the UI_URLs. Fortunately their env naming matches the path of the yaml but in upper case and with underscores. I think it's helpful to have the env var to be easily-copied, though


flows:
error:
ui_url: http://localhost:3000/error
Expand Down Expand Up @@ -59,6 +71,13 @@ selfservice:
in: header
- hook: session

verification:
use: code
lifespan: 48h
# Set though SELFSERVICE_FLOWS_VERIFICATION_UI_URL
ui_url: http://localhost:3000/verification
enabled: true

log:
level: debug
format: text
Expand All @@ -78,3 +97,8 @@ identity:
schemas:
- id: default
url: file:///etc/config/kratos/identity.schema.json

courier:
template_override_path: /etc/config/kratos/templates
smtp:
connection_uri: set-through-env-arg-COURIER_SMTP_CONNECTION_URI
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you able to set this dummy value because the environment variable overrides whatever we specify in the config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly, I think for dev mode, we should maybe just let it be the mailslurper url and add a comment as we do for SELFSERVICE_METHODS_LINK_CONFIG_BASE_URL above. I will do this now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

26 changes: 26 additions & 0 deletions packages/hash/external-services/kratos/kratos.prod.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@ selfservice:
password:
enabled: true

# Email sending diabled in production for now
# link:
# config:
# # The URL for verification emails are set through the link method
# # but we're using the code method, so we disable this method for usage.
# enabled: false
# # Set through SELFSERVICE_METHODS_LINK_CONFIG_BASE_URL
# base_url: http://localhost:3000/api/ory
# code:
# config:
# # and make sure to enable the code method.
# enabled: true

flows:
error:
ui_url: http://localhost:3000/error
Expand Down Expand Up @@ -59,6 +72,14 @@ selfservice:
in: header
- hook: session

# Email sending diabled in production for now
# verification:
# use: code
# lifespan: 48h
# # Set though SELFSERVICE_FLOWS_VERIFICATION_UI_URL
# ui_url: http://localhost:3000/verification
# enabled: true

log:
level: info
format: json
Expand All @@ -78,3 +99,8 @@ identity:
schemas:
- id: default
url: file:///etc/config/kratos/identity.schema.json
# Email sending diabled in production for now
# courier:
# template_override_path: /etc/config/kratos/templates
# smtp:
# connection_uri: set-through-env-arg-COURIER_SMTP_CONNECTION_URI
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Your verification code is: {{ .Code }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Hi,

you (or someone else) entered this email address when trying to recover access to an account.

However, this email address is not on our database of registered users and therefore the attempt has failed.

If this was you, check if you signed up using a different address.

If this was not you, please ignore this email.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Hi,

you (or someone else) entered this email address when trying to recover access to an account.

However, this email address is not on our database of registered users and therefore the attempt has failed.

If this was you, check if you signed up using a different address.

If this was not you, please ignore this email.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Account access attempted
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Hi,

please recover access to your account by clicking the following link:

<a href="{{ .RecoveryURL }}">{{ .RecoveryURL }}</a>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Hi,

please recover access to your account by clicking the following link:

{{ .RecoveryURL }}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Recover access to your account
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Hi,

you (or someone else) entered this email address when trying to recover access to an account.

However, this email address is not on our database of registered users and therefore the attempt has failed.

If this was you, check if you signed up using a different address.

If this was not you, please ignore this email.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Hi,

you (or someone else) entered this email address when trying to recover access to an account.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would make it less ambiguous for now what these emails are referring to if we refer to a "HASH account" rather than just "account". (this suggestion applies to most templates)

Suggested change
you (or someone else) entered this email address when trying to recover access to an account.
you (or someone else) entered this email address when trying to recover access to a HASH account.

cc @nonparibus for thoughts on the initial phrasing here

Copy link
Member

@vilkinsons vilkinsons Dec 13, 2022

Choose a reason for hiding this comment

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

Agreed "HASH account" preferable.

I would like to do a big pass at all of our transactional email copy soon.

Not necessarily as part of this PR, but could we consider how we might centrally collect all of our transactional email text in one place?

This could be creating an index pointing to the relevant lines of files like this, it could be a single directory that contains the actual emails themselves, or something like else (better) entirely. Either way we're not well set up for this at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nonparibus for now all the kratos transactional emails are in this folder https://github.com/hashintel/hash/tree/e98a86be821317929dfaa15edfd255af51222726/packages/hash/external-services/kratos/templates
for emails that we will send through HASH, we have to consider what we do to either co-locate them together or make them easily searchable. For now I think this kratos grouping is a nice start

Copy link
Contributor

Choose a reason for hiding this comment

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

Just want to flag that this suggestion also applies to the remaining email templates (including email subjects). Let me know if you intend on addressing this @thehabbos007

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, missed that this was for all: f710e62

Copy link
Member

Choose a reason for hiding this comment

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

Thanks -- wasn't aware we had the dir already. This is great.

I don't know what ultimate best-practice looks like here, but I would be happy with either:

  • somewhere I can go that links out to all the different collections of transactional emails (like this), provided we actually keep it up to date;
  • a single directory in which we store all transactional emails from across the app.

My sense is the latter probably makes marginally more sense, and will be easier to track.

I'll review/update Kratos email copy in a separate PR. Please make any adjustments between the two of you that you think make sense in the meantime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that a single directory would make most sense, it may be difficult to do currently without knowing how we structure mails in the HASH api. I'll make an index that links to the email templates in-use from /packages/hash/README.md where we already have a seciton on sending emails. Does that sound good for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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


However, this email address is not on our database of registered users and therefore the attempt has failed.

If this was you, check if you signed up using a different address.

If this was not you, please ignore this email.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Account access attempted
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Hi,

please recover access to your account by entering the following code:

{{ .RecoveryCode }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Hi,

please recover access to your account by entering the following code:

{{ .RecoveryCode }}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Recover access to your account
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Hi,

someone asked to verify this email address, but we were unable to find an account for this address.

If this was you, check if you signed up using a different address.

If this was not you, please ignore this email.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Hi,

someone asked to verify this email address, but we were unable to find an account for this address.

If this was you, check if you signed up using a different address.

If this was not you, please ignore this email.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Someone tried to verify this email address
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Hi,

please verify your account by entering the following code:<br />
{{ .VerificationCode }}<br />
or clicking the following link:<br />
<a href="{{ .VerificationURL }}" target="_blank">{{ .VerificationURL }}</a><br />

The link is valid for 48 hours.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Hi,

please verify your account by entering the following code:

{{ .VerificationCode }}

or clicking the following link:

{{ .VerificationURL }}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Please verify your email address
30 changes: 24 additions & 6 deletions packages/hash/frontend/src/pages/shared/ory-kratos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ import {
SettingsFlow,
VerificationFlow,
UiNodeInputAttributes,
UpdateLoginFlowBody,
UpdateRegistrationFlowBody,
UpdateRecoveryFlowBody,
UpdateSettingsFlowBody,
UpdateVerificationFlowBody,
} from "@ory/client";
import { isUiNodeInputAttributes } from "@ory/integrations/ui";
import { AxiosError } from "axios";
Expand Down Expand Up @@ -54,15 +59,15 @@ export type IdentityTraits = {
};

type Flows = {
login: LoginFlow;
recovery: RecoveryFlow;
registration: RegistrationFlow;
settings: SettingsFlow;
verification: VerificationFlow;
login: [LoginFlow, UpdateLoginFlowBody];
recovery: [RecoveryFlow, UpdateRecoveryFlowBody];
registration: [RegistrationFlow, UpdateRegistrationFlowBody];
settings: [SettingsFlow, UpdateSettingsFlowBody];
verification: [VerificationFlow, UpdateVerificationFlowBody];
};

type FlowNames = keyof Flows;
type FlowValues = Flows[FlowNames];
type FlowValues = Flows[FlowNames][0];

/**
* A helper function that creates an error handling function for some common errors
Expand Down Expand Up @@ -171,6 +176,19 @@ export const createFlowErrorHandler =
return Promise.reject(err);
};

export const gatherUiNodeValuesFromFlow = <T extends FlowNames>(
flow: FlowValues,
): Flows[T][1] =>
flow.ui.nodes
.map(({ attributes }) => attributes)
.filter((attrs): attrs is UiNodeInputAttributes =>
isUiNodeInputAttributes(attrs),
)
.reduce((acc, attributes) => {
const { name, value } = attributes;
return { ...acc, [name]: value };
}, {} as Flows[T][1]);

const maybeGetCsrfTokenFromFlow = (flow: FlowValues) =>
flow.ui.nodes
.map(({ attributes }) => attributes)
Expand Down
Loading