Skip to content

feat: add signup email security controls#3039

Open
HarshMN2345 wants to merge 12 commits into
mainfrom
feat-signup-email-security
Open

feat: add signup email security controls#3039
HarshMN2345 wants to merge 12 commits into
mainfrom
feat-signup-email-security

Conversation

@HarshMN2345
Copy link
Copy Markdown
Member

@HarshMN2345 HarshMN2345 commented May 11, 2026

What does this PR do?

Uploading image.png…

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

@HarshMN2345 HarshMN2345 requested a review from Meldiron May 11, 2026 06:31
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 11, 2026

Greptile Summary

This PR adds three new email-security policy controls (block free emails, require canonical emails, block disposable emails) to the auth security settings page, and broadly refactors all security, services, protocols, and auth-method components to read from dedicated policy objects (fetched via getPolicy()) rather than flat fields on the Models.Project model. It also migrates several SDK enum references to their renamed counterparts (ProjectServiceId, ProjectProtocolId, ProjectAuthMethodId, etc.) and restructures the onboarding progress tracking to use layout-level platform/key list fetches.

  • New email-policy card (updateSignupEmailSecurity.svelte): uses onMount for state initialization (unlike sibling components that initialize directly in $state), always fires all three API calls on submit regardless of which setting changed, and hardcodes the first submit enum in error tracking — making it impossible to identify which of the three calls failed from analytics data.
  • Security page load (+page.ts): fetches all 12 policies in a single Promise.all; the three new email-policy IDs are locally defined and force-cast as ProjectPolicyId, so a backend that doesn't yet serve them will cause the entire security page to fail to load.
  • Org page (organization-[organization]/+page.ts): adds a sequential await listPlatforms() per project inside a for loop, replacing what was previously a zero-extra-request operation — an org with many projects now incurs N serial round-trips, and a single failing project tears down the entire overview.

Confidence Score: 2/5

Multiple functional regressions exist: the security page can fail entirely if any email-policy endpoint is unrecognized, the org overview can break if one project platform fetch fails, and service/protocol/auth-method toggles can silently appear as indeterminate if backend arrays are incomplete.

The security page's Promise.all over 12 policies — three of which use force-cast IDs not in the SDK enum — will reject and leave all security controls inaccessible if the backend doesn't serve the new endpoints. The org page's sequential per-project listPlatforms loop has no error isolation, so a single unavailable project region breaks the entire org overview.

The security page load (auth/security/+page.ts), the organization overview page load (organization-[organization]/+page.ts), and the three store files (project-services.ts, auth-methods.ts, project-protocols.ts) need the most attention before merging.

Important Files Changed

Filename Overview
src/routes/(console)/project-[region]-[project]/auth/security/updateSignupEmailSecurity.svelte New email-policy settings card with issues: onMount initialization creates a pre-hydration window where hasChanges is incorrectly true; partial-failure leaves server state inconsistent; error analytics always attribute to the canonical-email call; and EnabledPolicy type is duplicated from +page.ts.
src/routes/(console)/project-[region]-[project]/auth/security/+page.ts New page-load function fetches all 12 policies in a single Promise.all; the three email-policy IDs are locally defined and force-cast as ProjectPolicyId, so if the backend doesn't serve them the entire security page fails to load.
src/routes/(console)/organization-[organization]/+page.ts Adds sequential await listPlatforms() inside a for loop — N serial round-trips instead of one parallel batch; a single failing project tears down the entire org overview.
src/routes/(console)/project-[region]-[project]/+layout.ts Switches project load from sdk.forConsole.projects.get to projectSdk.get(); adds parallel listPlatforms and listKeys fetches for the onboarding store and shell progress card.
src/routes/(console)/project-[region]-[project]/store.ts Rewrites onboarding derived store from project-embedded arrays to page.data.platforms.total and page.data.keys.total, now sourced from layout-level fetches.
src/lib/stores/project-services.ts Migrates from flat boolean fields to a Map built from project.services[]; missing entries now silently yield null instead of a clear boolean, which may render toggles as indeterminate.
src/lib/stores/auth-methods.ts Same pattern as project-services.ts — migrates to ProjectAuthMethodId and a Map over project.authMethods[]; absent entries silently become null.
src/lib/stores/project-protocols.ts Same Map-based migration for protocols; absent entries become null silently.
package.json Pins @appwrite.io/console to a bare commit hash on pkg.vc without an integrity checksum, replacing the versioned npm release.

Reviews (14): Last reviewed commit: "Use project key scope type in selector" | Re-trigger Greptile

Comment thread src/routes/(public)/auth/preview/+page.svelte Outdated
Comment thread src/routes/(console)/project-[region]-[project]/overview/api-keys/[key]/store.ts Outdated
import type { ProjectKeyScopes, Scopes } from '@appwrite.io/console';

let { scopes = $bindable([]) }: { scopes: Scopes[] } = $props();
type ScopeValue = ProjectKeyScopes | Scopes;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, not good. Why two? What is differene between ProjectKeyScopes and Scopes?

DId you see anything else like this we should think about, from quality perspective in new SDK?

return {
platforms: {
platforms: project.platforms,
total: project.platforms.length
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why did we remove total? new list API should have proper pagination no problem

import { user } from '$lib/stores/user';
import { organization } from '$lib/stores/organization';
import { ID, Scopes } from '@appwrite.io/console';
import { ID, ProjectKeyScopes as Scopes } from '@appwrite.io/console';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not big fan of alias here, we could instead rename to new name.

Lets check for this in all files

Comment on lines +131 to +132
platforms: platformList,
keys: keyList,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need platforms and keys for layout, or can layout render without it - and then we load those in releavnt +page.ts (for dashboard)?


loadAvailableRegions(project.teamId)
]);
const [org, rawRegionalConsoleVariables, rolesResult, , platformList, keyList] =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Something here feels like a bug, notice two commas , ,

</svelte:head>

<Onboard platforms={page.data.project.platforms} pingCount={page.data.project.pingCount} />
<Onboard platforms={page.data.platforms.platforms} pingCount={page.data.project.pingCount} />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

platforms platforms isnt correct I think 🤔 Why not just page.data.platforms?

I might be wrong tho. Is there other places where we have this double-name pattern? Maybe something iwth pagination?

Comment on lines +9 to +21
export type AuthProvider = {
$id: string;
key: string;
name: string;
appId: string;
secret: string;
enabled: boolean;
} & Record<string, unknown>;

type Args = {
region: string;
projectId: string;
provider: Models.AuthProvider;
provider: AuthProvider;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need cusotm type, which also accepts anything on top of it?

AuthPasswordHistoryUpdate = 'submit_auth_password_history_limit_update',
AuthPasswordDictionaryUpdate = 'submit_auth_password_dictionary_update',
AuthPersonalDataCheckUpdate = 'submit_auth_personal_data_check_update',
AuthCanonicalEmailsUpdate = 'submit_auth_canonical_emails_update',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we renamed, its aliased emails, not canonical emails

let percentage = 33;

if (platforms.length > 0 && pingCount === 0) {
if ((platforms?.total ?? 0) > 0 && pingCount === 0) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Interesting change here - before you got all, now you get a page.

People with 25+ platforms might complain its not working.

We need to introduce pagination.

Same for API keys, mock phone numbers

Comment thread src/lib/stores/page.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Curious why we added this

Comment thread src/routes/(console)/organization-[organization]/+page.svelte
on:click={handleCreateProject}>
{#each data.projects.projects as project}
{@const projectPlatforms =
(project as Models.Project & { platforms: Array<{ type: string }> })
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Project no longer has platforms, this wont work. Adding platforms forcefully to the type is like doing any - bad practice

href={`${base}/project-${project.region}-${project.$id}/overview/platforms`}>
<svelte:fragment slot="eyebrow">
{project?.platforms?.length ? project?.platforms?.length : 'No'} apps
{projectPlatforms.length ? projectPlatforms.length : 'No'} apps
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Proper pagination might be needed here - diffrerenciating .length (current page)and.total` (total count)

Comment on lines +49 to +51
(project as Models.Project & { platforms: Array<{ type: string }> }).platforms = (
await sdk.forProject(project.region, project.$id).project.listPlatforms()
).platforms;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same comment as above - platforms are not available here, this wont work. Shouldnt.

Comment on lines +10 to +30
const [
membershipPrivacyPolicy,
passwordDictionaryPolicy,
passwordHistoryPolicy,
passwordPersonalDataPolicy,
sessionAlertPolicy,
sessionDurationPolicy,
sessionInvalidationPolicy,
sessionLimitPolicy,
userLimitPolicy
] = await Promise.all([
projectSdk.getPolicy({ policyId: ProjectPolicyId.Membershipprivacy }),
projectSdk.getPolicy({ policyId: ProjectPolicyId.Passworddictionary }),
projectSdk.getPolicy({ policyId: ProjectPolicyId.Passwordhistory }),
projectSdk.getPolicy({ policyId: ProjectPolicyId.Passwordpersonaldata }),
projectSdk.getPolicy({ policyId: ProjectPolicyId.Sessionalert }),
projectSdk.getPolicy({ policyId: ProjectPolicyId.Sessionduration }),
projectSdk.getPolicy({ policyId: ProjectPolicyId.Sessioninvalidation }),
projectSdk.getPolicy({ policyId: ProjectPolicyId.Sessionlimit }),
projectSdk.getPolicy({ policyId: ProjectPolicyId.Userlimit })
]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lets instead use listPolicies

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note for self, @Meldiron should double check copy

import { oAuthProviders } from '$lib/stores/oauth-providers';
import { sdk } from '$lib/stores/sdk';
import { AuthMethod as MethodId, type Models } from '@appwrite.io/console';
import { ProjectAuthMethodId as MethodId } from '@appwrite.io/console';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would avoid alias, shouldnt be needed

Comment on lines 24 to 27
export const onboarding = derived(
project,
($project) => $project?.platforms?.length === 0 && $project?.keys?.length === 0
page,
($page) => $page.data.platforms?.total === 0 && $page.data.keys?.total === 0
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 onboarding always evaluates to false

$page.data.keys is never populated by any page load function in this PR — no +page.ts or +layout.ts loads a key list into page.data.keys. Because of the && condition, keys?.total === 0 evaluates to undefined === 0false on every route, making the whole expression permanently false. As a result, create.svelte never calls invalidate(Dependencies.PROJECT) after a new API key is created on a fresh (zero-platforms, zero-keys) project, so the onboarding progress card does not advance when an API key is the qualifying action.

Comment thread src/routes/(console)/project-[region]-[project]/get-started/+page.ts Outdated
Comment thread src/routes/(console)/organization-[organization]/+page.ts
Comment thread src/routes/(console)/organization-[organization]/+page.ts Outdated
Comment on lines +11 to +16
const services = new Map(project?.services?.map((service) => [service.$id, service.enabled]));

const rows: Service[] = [
{
label: 'Account',
method: ServiceId.Account,
value: project?.serviceStatusForAccount ?? null
method: ProjectServiceId.Account,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Silent null fallback masks missing service entries

The same pattern applied across project-services.ts, auth-methods.ts, and project-protocols.ts: if project.services (or .authMethods / .protocols) is absent or doesn't contain an entry for a given ID, services.get(...) returns undefined and ?? null yields null for that toggle. Previously these read flat boolean fields (project?.serviceStatusForAccount), so null only occurred when the entire project was null. Now any service/method/protocol missing from the backend array silently renders as indeterminate — and any UI that treats null the same as false (disabled) would silently suppress what may actually be an enabled setting.

@HarshMN2345 HarshMN2345 force-pushed the feat-signup-email-security branch from 2708bc4 to 13382b6 Compare May 12, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants