-
Notifications
You must be signed in to change notification settings - Fork 158
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
Migration to 2.0 Problem -- RedirectUrisSignOut #82
Comments
Looking closely at the auth@edge 2.0 code, I think it can go wrong if you:
Which is indeed the scenario you're in, looking at that piece of CFN you pasted. You can unblock yourself by providing AlternateDomainNames or by setting CreateCloudFrontDistribution to true. Meanwhile, I will fix the code, to make this scenario work again too. Question: is this just a test or do you actually want to deploy like this? Because in this scenario you always have to manually update the redirect URI's in the User Pool Domain, after doing the deployment. |
This isn't our most common deploy scenario -- the common usage is to use an So we would like to be able to both create the cloudfront in the top level stack, setting Unfortunately it looks like a@e needs the Cloudfront default URL, and CloudFront needs to know the URLs of the a@e lambdas so there is still a circularity. Failing a true solution it would be very welcome to restore the 1.2 capability to allow no AlternateDomainName and CreateCloudFrontDistribution to False, then manually update the user pool. Eventually we may rewrite to avoid the top level CloudFormation stack altogether in favor of our internal cloud API tool called mu -- but that's a long term prospect. |
Yes that can only be solved by a custom resource in the top level stack. But the custom resource implementation can be borrowed from a@e, so it will be simple actually. But the a@e custom resource handler arn needs to be made a stack output, so you can refer to it (a 1 min change to a@e) |
Funny, I was out walking the dog and the same idea occurred to me. There's already a That would be a great solution ... and I hope it's something you might do. I'm seeing it like this -- just to get it on paper:
In addition, we can add functionality to significantly make the stacks more independent as you suggest:
Is that what you had in mind? Is that possible? If so I'd be happy to test it out. R. |
LOL! Yeah that's it in principle––but I mean the UserPoolClientUpdate custom resource to be exact. I need to have a step back to see how this resource is used now and if the plan really makes sense, but I think it does. I'll keep you posted. If you wanna do it by the way, that's fine too. I can provide guidance while you code the PR (if you need it) |
I'd rather you did if you're willing ... I'm pretty snowed on the app itself, and don't have any experience compiling a SAM module. |
This should be it: #83 |
OK you should now be able to add a resource like this to the top-level stack:
|
v2.0.1 in the SAR |
It must be about a million o'clock in NE -- thanks! I'll test it right away |
The log group error might be a red herring––the real error looks to be "invalid_scope" How are you passing the scopes to the custom resource? It expects them here as a list of strings (not as a CommaDelimitedList):
That list needs to be the same as what you passed to Auth@Edge (if you did not pass anything it needs to be the same as the Auth@Edge default). But then it is probably easier to not provide the scopes when invoking the custom resource––and keep the ones from auth@edge. So, this should also work:
|
Well, not having a lot of luck here.
and the stack deployed, but the run failed, as noted. I believe the two formats should be the same ... but apparently not. Substituting the corrected string format in both the UserPoolClient and the UserPoolClientUpdate allowed for a deploy, but I get the identical error on scope. Adding the additional scope parameter So, once I got past the scope error I was able to log into Cognito and change my password, but then encountered a new error in parseAuth:
Here are the two stanzas as they currently are ... note that the parameter names are different which seems to be required ... UserPoolClient:
UserPoolClientUpdate:
By the way, the
|
By the way2 , the scenario 1 (no custom resource, just cloudfront=false and no alternate domain) also fails, this time with an unspecified error after login password entered to cognito: |
Mmm :| Can you paste your entire CFN template here, I'll try to reproduce |
Preparing a stripped version as the original accesses buckets, etc. |
Edited, simplified. Gives the error
|
Only stack parms needed are the bucket name and stack name |
I've found the culprit: in your template change GenerateSecret to false and redeploy and 🎉 it works. Or, leave it to true, but disable SPA mode then for Auth@Edge: EnableSPAMode = false I'll update the docs to point this out clearly (and fix the reuse example which was wrong). Note: I found the cause by setting LogLevel to "debug" for Auth@Edge and checking what happened in parseAuth logs. |
Thanks! That's great news! Starting testing now. Some interesting questions:
Testing away, exciting! |
Updated this example: example-serverless-app-reuse/reuse-with-existing-user-pool.yaml About your Q's:
Have a read of this: SPA mode or Static Site mode? For SPAs it's not useful to have a Client Secret, as anyone can do "view source" and see it.
It's a param to the app, just as CreateCloudFrontDistribution and EnableSPAMode: cloudfront-authorization-at-edge/template.yaml Lines 122 to 131 in 78867bb
To find the logs you have to go through hoops a bit, as Lambda@Edge logs get their own special log groups, in the region where they end up running (the "normal" log group of the Lambda would not show anything). Easiest way to find them is to go to the CloudFront monitoring dashboard, find the function invocations and jump to the log in the right region there. |
Thanks, makes sense. I took SPA mode too literally I think. For whatever reason I do not see the updates to example-serverless-app-reuse/reuse-with-existing-user-pool.yaml in the master branch. Continuing testing
There may be an issue on the 3rd test, which is the app without custom resources and a manual update to the Application Client callback URLs. Will pursue and get back |
Still an issue on the final test. Works like this:
This doesn't look like it's coming from the lambdas, rather from Cognito |
I can't reproduce this error. For me deploying the below template, and after deployment changing the redirect URL's, works fine. So what is different in your case?
|
Thanks, Otto. I've gone back to that install after doing nothing to it over the weekend, and it is now working! Very peculiar that it stabilized without action. Let's pass over this particular rabbit-hole and I'l raise it again if it becomes an issue. Looks like 2.01 is now a going concern ... will work on some of the cookie injection magic it brings. |
Working on migrating our application to a@e 2.0 to look further at #81 and other features, and run into a roadblock. The demonstration stack for 2.0 that illustrates how to define Cognito UserPool etc. in the parent stack works fine but ...
I've pulled the Cognito User Pool and other related artifacts into the main stack, and successfully created all artifacts until we get to the calls to the nested a@e stack. In creating the framework I see:
Embedded stack arn:aws:cloudformation:us-east-1:REDACTED:stack/ae200d-LambdaEdgeProtection-L97QEYQVDZLR/4cb7b300-ef9e-11ea-ace7-126c97cb5bc1 was not successfully created: Cannot export output RedirectUrisSignOut. Exported values must not be empty or whitespace-only.
My call to the nested stack which worked fine in 1.2 has changed only in minor ways ... I've added parameters for the UserPoolArn and UserPoolClientId I've created in the parent stack, just as the a@e example for 2.0 does. The failing call looks like this:
A visit through the code shows that RedirectUrisSignOut is mainly referenced in
src/cfn-custom-resources/user-pool-client/index.ts
and in the main template, suggesting that maybe I've got the dependencies wrong, but they seem obvious ... an explicit depends on the UserPoolDomain, and implicit ones for the UserPoolClient and UserPool.Arn. All we're really doing here is adding the HttpHeaders parameter from the main template.Another possibility -- because this example does not use custom domains, but just relies on the CloudFront URL, as does the example, is some problem with the sentinel values -- but I left them alone, as they are in the example.
Any idea what might be going on?
The text was updated successfully, but these errors were encountered: