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(amplify-category-auth): add HostedUI custom domain in metadata #6499

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

vic-blt
Copy link
Contributor

@vic-blt vic-blt commented Jan 28, 2021

Fix #1880

HostedUIDomain equals to either the domain prefix or the custom domain defined in the user pool.
However we need to differentiate a custom domain from a domain prefix for the config put in aws-exports.js

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@vic-blt vic-blt requested a review from a team as a code owner January 28, 2021 13:00
@codecov
Copy link

codecov bot commented Jan 28, 2021

Codecov Report

Merging #6499 (a21f00e) into master (39dfd27) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #6499    +/-   ##
========================================
  Coverage   57.28%   57.28%            
========================================
  Files         479      479            
  Lines       21695    21695            
  Branches     4096     4312   +216     
========================================
  Hits        12429    12429            
+ Misses       8405     8385    -20     
- Partials      861      881    +20     
Impacted Files Coverage Δ
...c/provider-utils/awscloudformation/import/index.ts 5.08% <ø> (ø)
...mplify-frontend-ios/lib/frontend-config-creator.js 16.66% <0.00%> (ø)
packages/amplify-util-mock/src/api/api.ts 0.00% <0.00%> (ø)
packages/graphql-mapping-template/src/print.ts 35.29% <0.00%> (ø)
packages/amplify-util-mock/src/storage/storage.ts 0.00% <0.00%> (ø)
...ges/amplify-util-mock/src/CFNParser/stack/index.ts 0.00% <0.00%> (ø)
...es/amplify-util-mock/src/api/resolver-overrides.ts 0.00% <0.00%> (ø)
...es/graphql-transformer-core/src/stripDirectives.ts 35.29% <0.00%> (ø)
.../amplify-cli-core/src/state-manager/pathManager.ts 67.44% <0.00%> (ø)
.../amplify-util-mock/src/utils/lambda/loadMinimal.ts 0.00% <0.00%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39dfd27...a21f00e. Read the comment docs.

@jhockett jhockett added the pending-review Pending review from core-team label Jan 28, 2021
@doc-l
Copy link

doc-l commented Apr 28, 2021

Any ETA on when this can be merged? This would be great and it looks like the work is already done...

@doc-l
Copy link

doc-l commented Jul 21, 2021

bump?

@vic-blt
Copy link
Contributor Author

vic-blt commented Jan 31, 2022

@edwardfoyle any news on this ?

@aws-eddy
Copy link
Contributor

Hey @vic-blt, would you be interested in continuing to work on this PR? If so, please rebase this onto dev by running git rebase dev. We will help you with getting this PR to the finish line 🏁

@aws-eddy aws-eddy added the pending-response Issue is pending response from the issue author label May 14, 2023
@vic-blt vic-blt force-pushed the master branch 2 times, most recently from 45b3caa to 9271ca2 Compare May 14, 2023 15:42
HostedUIDomain equals to either the domain prefix or the custom domain defined in the user pool.
However we need to differentiate a custom domain from a domain prefix for the config put in
aws-exports.js

fix aws-amplify#1880
@codecov-commenter
Copy link

Codecov Report

Merging #6499 (d1ac574) into dev (ec9a2ba) will increase coverage by 47.58%.
The diff coverage is 46.42%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@             Coverage Diff             @@
##              dev    #6499       +/-   ##
===========================================
+ Coverage    0.00%   47.58%   +47.58%     
===========================================
  Files        1296      843      -453     
  Lines      149743    38370   -111373     
  Branches     1296     7852     +6556     
===========================================
+ Hits            0    18257    +18257     
+ Misses     148447    18481   -129966     
- Partials     1296     1632      +336     
Impacted Files Coverage Δ
...ify-category-function/src/events/prePushHandler.ts 33.33% <0.00%> (+33.33%) ⬆️
...lify-category-hosting/lib/S3AndCloudFront/index.js 89.65% <ø> (+89.65%) ⬆️
...s/amplify-cli-core/src/errors/amplify-exception.ts 82.35% <ø> (+82.35%) ⬆️
packages/amplify-cli-core/src/types.ts 100.00% <ø> (+100.00%) ⬆️
packages/amplify-cli/src/commands/console.ts 0.00% <0.00%> (ø)
packages/amplify-cli/src/commands/pull.ts 0.00% <0.00%> (ø)
...mplify-frontend-ios/lib/frontend-config-creator.js 15.02% <0.00%> (+15.02%) ⬆️
...ider-awscloudformation/src/display-helpful-urls.ts 25.42% <0.00%> (+25.42%) ⬆️
...y-provider-awscloudformation/src/push-resources.ts 20.73% <0.00%> (+20.73%) ⬆️
...c/extensions/amplify-helpers/auth-notifications.ts 41.00% <4.34%> (+41.00%) ⬆️
... and 21 more

... and 1275 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@vic-blt
Copy link
Contributor Author

vic-blt commented May 14, 2023

@evcodes I rebased onto dev.

@aws-eddy aws-eddy removed the pending-response Issue is pending response from the issue author label May 15, 2023
@aws-eddy aws-eddy self-requested a review May 15, 2023 16:47
@aws-eddy
Copy link
Contributor

Thank you, I am running the changes through our e2e test suite

Copy link
Contributor

@aws-eddy aws-eddy left a comment

Choose a reason for hiding this comment

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

This looks good to me. Thank you!

@aws-eddy aws-eddy added the do-not-merge PRs that are approved and green, but should not be merged yet label May 15, 2023
@edwardfoyle edwardfoyle removed their assignment May 23, 2023
@aws-eddy aws-eddy self-assigned this May 24, 2023
@fossamagna
Copy link
Contributor

@evcodes @goldbez
When do you plan to merge this RP?
This RP change only supports AMPLIFY auth import.
If this RP is merged, I would willing to create an RP that adds support for custom domains via amplify auth add/amplify auth update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge PRs that are approved and green, but should not be merged yet pending-review Pending review from core-team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom domain support with Cognito
8 participants