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

feat(passport): account mask address #2743

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

szkl
Copy link
Contributor

@szkl szkl commented Oct 24, 2023

Description

Introduces mask account feature for email account type. The authorization page
has a toggle switch to mask an email account. The same email account is replaced
by the masked account in the connected accounts too. Only masked email account
is set in the authorization persona data. The user can view the source account
of a granted claim to an application in the passport authorization listing.

The mask email account address is generated when user turns on the switch. An
email account gets a designated mask address for each application. The mask
account is associated with the identity of the source account. The mask email
account is not visible to the user except for viewing the authorization claim
data.

Various modules and components has changes to provide functionality for the
complete the feature. This pull request also has some utility improvement
changes.

The removed storybook modules were failing.

Related Issues

Testing

  • Manual

Checklist

  • I have read the CONTRIBUTING guidelines
  • I have tested my code (manually and/or automated if applicable)
  • I have updated the documentation (if necessary)

@szkl szkl marked this pull request as draft October 24, 2023 03:21
@szkl szkl self-assigned this Oct 24, 2023
@szkl szkl added the enhancement Indicates new feature requests label Oct 24, 2023
@szkl szkl force-pushed the feat/passport/account-mask-address branch 3 times, most recently from 8f37929 to fe6c321 Compare October 24, 2023 08:26
@szkl szkl force-pushed the feat/passport/account-mask-address branch from fe6c321 to 19514d7 Compare October 24, 2023 08:31
@@ -171,12 +173,14 @@ export const ClaimsMobileView = ({ scopes }: { scopes: any[] }) => {
const RowView = ({
account,
appAskedFor,
masked = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be used in the mobile view

@@ -527,15 +532,19 @@ export const ClaimsWideView = ({ scopes }: { scopes: any[] }) => {
>
{appAskedFor}
</Text>
{masked ? <EmailMaskedPill /> : null}
Copy link
Contributor

Choose a reason for hiding this comment

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

For this kind of condition there's also the && pattern: {masked && <EmailMaskedPill />}

if (!maskEmail) return
if (selectedEmail?.mask) return
setMaskEmailCallback()
})
Copy link
Contributor

Choose a reason for hiding this comment

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

This should have a dependency array, either an empty one: [] if its intended to run only on component mount, or possibly a [maskEmail, selectedEmail] one if its intended to run when one of the dependencies change.

}),
}

setDataForScopes(newState)
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be a good use case for:
setDataForScopes(prevState => ({ ...prevState, newState }));

</Pill>
)

export const EmailMaskedPill = () =>
Copy link
Contributor

Choose a reason for hiding this comment

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

A more react-ish way would be to do:

export const EmailMaskedPill = () => (
  <BaseEmailPill title="Masked" IconComponent={TbShield} />
);

export const EmailUnmaskedPill = () => (
  <BaseEmailPill title="Unmasked" IconComponent={TbShieldOff} />
);

@szkl szkl force-pushed the feat/passport/account-mask-address branch 4 times, most recently from 706b2ac to 74f5b9e Compare October 26, 2023 00:22
@szkl szkl force-pushed the feat/passport/account-mask-address branch from 74f5b9e to de3f77b Compare October 26, 2023 16:03
Base automatically changed from feat/core/email-relay to main October 26, 2023 17:13
@szkl szkl force-pushed the feat/passport/account-mask-address branch 2 times, most recently from cb6acf5 to 2e8f8f9 Compare October 29, 2023 19:58
@szkl szkl force-pushed the feat/passport/account-mask-address branch 7 times, most recently from 1daa218 to dbde928 Compare November 9, 2023 07:46
@szkl szkl force-pushed the feat/passport/account-mask-address branch from dbde928 to e0f5e45 Compare November 11, 2023 03:40
@szkl
Copy link
Contributor Author

szkl commented Nov 25, 2023

I've addressed most of the comments and only replied to a few. You can take another look.

@szkl szkl force-pushed the feat/passport/account-mask-address branch 10 times, most recently from f090949 to b4e2566 Compare November 30, 2023 23:56
@szkl
Copy link
Contributor Author

szkl commented Nov 30, 2023

It is time to bring this home!

@szkl szkl force-pushed the feat/passport/account-mask-address branch 6 times, most recently from 25d6a71 to b596cfe Compare December 1, 2023 21:18
@szkl szkl force-pushed the feat/passport/account-mask-address branch 2 times, most recently from ab22582 to ee7e4b7 Compare December 4, 2023 22:57
@szkl szkl force-pushed the feat/passport/account-mask-address branch 2 times, most recently from 32ce4ad to a5a7a70 Compare December 7, 2023 05:35
@szkl szkl force-pushed the feat/passport/account-mask-address branch from a5a7a70 to eb79c4a Compare December 8, 2023 14:40
@betimshahini betimshahini merged commit 5625048 into main Dec 8, 2023
18 of 20 checks passed
@betimshahini betimshahini deleted the feat/passport/account-mask-address branch December 8, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(platform): Masked email relay
3 participants