Open PeteDuncanson opened 2 years ago
@PeteDuncanson thanks for bringing this up. We will get on triaging this. While we do that, I just wanted to make a callout, the reason we might be looking for ?code
instead of what you are asking for in expected behavior would be for 3rd party /federated auth use cases. We gotta look deeper on this since we understand weirdness in implementation you are doing to get around.
Having the same issue with gsuite auth enabled. This is a regression of a previous issue that was fixed, can't find it currently but will add if i do.
Having the same issue with bitbucket oauth. I downgraded to latest-3 in the meantime. Hopefully this is fixed soon.
@thejuan @kieran-bellew When did u start seeing the issue ? Could you point what upgrade triggered this?
@ashika01 (I'm working with @kieran-bellew) we first saw it after upgrading aws-amplify/auth
from 1.6.3 -> 4.3.16 and aws-amplify/core
from 1.3.3 -> 4.3.8. After rolling back both packages to 3.4.34 and 3.8.24 respectively we stopped seeing the issue. The issue we encountered was not with ?code=foo
in our path, but rather #auth_token=foo
. It caused the same behavior described by @PeteDuncanson. Oddly enough, when switched to #auth-token=foo
we no longer saw the issue.
@colehendo Thank you for the info. Let me look into it and get back to you.
Have you tried any version 4.x.x to see if it works?
Also, @colehendo if u have a sample running out in github, point it out here. Helps speed up debugging :)
@ashika01 I don't have the code in github, but I can describe our process.
We have a component on our site which requires Bitbucket authentication to access. The user clicks on the link in our site (built using Angular), and get rerouted to https://bitbucket.org/site/oauth2/authorize?client_id=<CLIENT_ID>&response_type=token
.
That url then automatically routes them back to https://my-site.com/bitbucket-redirect#auth_token=12345
. On that redirect back to our site, the url with the /bitbucket-redirect
path is displayed in the browser. Before the Angular component associated with the bitbucket-redirect
path can even construct (which pulls and stores the value associated with auth_token
), the user is redirected to the root path at https://my-site.com/
@PeteDuncanson I am trying to reproduce the issue, I would like to share somethings I found so far:
http://localhost:8080/#callback?code=100
, but if there is an attempt to reroute /callback
back to the index.html
. Then the code handler will be triggered.Potentially related to https://github.com/aws-amplify/amplify-js/pull/8166, but overall this working code flow needs careful thinking as we also need to support react native use case. Will do more study on this.
Hello @hkjpotato any news about this issue ?
Hello,
we are having a very similar issue. For us it's happening when we have a ?error=anything_here
in the url. I also tried ?code=
and also got the redirect.
We are using aws-amplify 4.3.12
.
Any news on this? For now I would tackle the error and try to solve it.
hey, any updates on this?
Still broken on aws-amplify@4.3.27
We got the same issue with the lastest version.
Anyone can help to merge this PR? https://github.com/aws-amplify/amplify-js/pull/9370
Thanks
It's worth mentioning that this issue causes problems for apps that use Cognito and SMART on FHIR together. e.g. the library https://github.com/smart-on-fhir/client-js, which relies heavily on the code parameter.
I believe this to be the commit that causes the issue: https://github.com/aws-amplify/amplify-js/commit/d4f8fe65fb6d54400f5bc1a6ee85a595f2fdf392
I'm facing the same problem, any news on where this https://github.com/aws-amplify/amplify-js/pull/9370 will be merge?
Just sent a followup to the PR creator to see if they can make updates based on recommendations from our team.
Same problem here.
Since cognito doesn't allow me to choose the scopes from Google, I ended up scrapping third party auth altogether. However, once you are in the app, there is the option to "sync your calendar". The Google auth workflow returns the code keyword and cognito picks it up, despite the fact I don't have third party auth enabled.
@abdallahshaban557 How long to wait before it's safe to assume the original PR creator will not see it through? It's a pretty serious issue, is there someone on the core team that can start the ball rolling again? This bug has been in affect for more than a year now.. (since this commit https://github.com/aws-amplify/amplify-js/commit/d4f8fe65fb6d54400f5bc1a6ee85a595f2fdf392)
Hi @timharsch - I am asking our team to check what needs to be done to fix this further. We will get back to you when we have more information. FYI @nadetastic @tannerabread @cwomack
@abdallahshaban557 Thanks for looking into this. It's actually a bug so it should be fixed by the bug creator instead of someone else. :) I'm pretty sure because it was working perfectly but suddently broken from a version.
@abdallahshaban557 Any update on this one? This remains a pretty serious bug and has been creating a lot of issues for us.
If it can help, I'm using this code to avoid the problem, waiting for the fix to be release
import { Auth } from '@aws-amplify/auth'
const _handleAuthResponse = Auth._handleAuthResponse.bind(Auth)
Auth._handleAuthResponse = (url) => {
const configuration = Auth.configure()
if (!url.includes(configuration.oauth.redirectSignIn)) return
return _handleAuthResponse(url)
}
If it can help, I'm using this code to avoid the problem, waiting for the fix to be release
import { Auth } from '@aws-amplify/auth' const _handleAuthResponse = Auth._handleAuthResponse.bind(Auth) Auth._handleAuthResponse = (url) => { const configuration = Auth.configure() if (!url.includes(configuration.oauth.redirectSignIn)) return return _handleAuthResponse(url) }
@asalzillo-treedom I tried this workaround and the default _handleAuthResponse still runs, then it hits this code after it already did a redirect. Does this need to go in a particular place?
@brandon-lango i have added it at the startup of the app as first think, before the configure of amplify
Hello. We are aware of this issue. We are reviewing a PR from an external contributor at the moment.
Still an open bug.
it is a kinda big deal for us, how to prevent Cognito from changing URLs that have a code
query param?
For reference, this is the commit responsible for the issue. https://github.com/aws-amplify/amplify-js/commit/d4f8fe65fb6d54400f5bc1a6ee85a595f2fdf392
I tried raising this issue with our AWS account manager, who had a solutions architect try to get the status from the core team. I was told that the response was that the issue is slated to be fixed in V6 release. I don't put a lot of faith in that response though because I also asked that someone from the team update this issue with a milestone label, and update the community.
Hi @timharsch - we have this issue high on our tracking list for v6, we understand the change needed, but do not want to break existing customers. We will update this issue when we have an ETA on our release! We are taking the concerns of our community to heart on resolving this pain! Please let us know if you have any further questions/concerns.
Here is my work around in the meanwhile.
/**
* This is a hack to fix the issue with the Amplify library treating all incoming urls as OAuth responses.
* https://github.com/aws-amplify/amplify-js/issues/9208#issuecomment-1309890756*
*/
// @ts-ignore
const _handleAuthResponse = Auth._handleAuthResponse.bind(Auth);
// @ts-ignore
Auth._handleAuthResponse = (url: string) => {
const configuration = Auth.configure();
// @ts-ignore
if (!url.includes(configuration.oauth?.redirectSignIn)) return;
return _handleAuthResponse(url);
};
Amplify.configure(awsExports);
Any updates on this? This is still a pretty big issue.
Also, FYI, if you're using the workaround proposed by @alexkates, the latest version it works on is 5.3.6, afterwards Auth.ts seems to have been completely refactor so it doesn't work.
I'm still on version 5.0.24
The developer preview for v6 of Amplify has officially been released with improvements Authentication and much more! Please check out our announcement and updated documentation to see what has changed.
This issue should be resolved within the dev preview and upcoming General Availability for Amplify v6, but let us know with a comment if there are further issues.
Please note that the method has changed from Auth.federatedSignIn()
to signInWithRedirect()
For those following, I'd recommend checking the upgrade docs https://docs.amplify.aws/javascript/build-a-backend/troubleshooting/migrate-from-javascript-v5-to-v6/
I have migrated to v6 and this bug is still happening.
All query params are being removed, not just ?code=.
Because the new way functions are imported in v6 (i.e. import { } from 'aws-amplify/auth'
) I can't use the hacks provided here to override Auth._handleAuthResponse
HI @Bar-Security do you mind opening a new issue with details and context so I may be able to better assist you?
Our team has found that upgrading to v6 is a non-trivial task for us. We first need to make our whole app webpack 5 compatible before we can use v6, since v6 now has a dependency on webpack 5 (https://github.com/aws-amplify/amplify-js/blob/b092100525e328247a814b6684ae4735ce3fc8da/package.json#L119). Is it possible to take the fix from v6 and backport it to v5? Can you point to the code, and/or tests, or maybe the PR, in v6 that solves the issue?
Issue already closed but fix still not released. Why?
@nadetastic please reopen the issue. The problem persists..
Reopening this issue as we investigate this further on both v5 and v6.
Hi @timharsch, cc @Bar-Security
We are still unable to reproduce this issue with v6 of aws-amplify
- here are the reproduction steps we have followed:
signInWithRedirect()
/?code=1234
to app's url which remains and isn't swallowedSince you are still seeing this issue with v6, do you mind opening a new issue with additional context and reproduction steps in order to better assist you?
I could also reproduce the behaviour mentioned by @Bar-Security and have opened a fresh issue as suggested by @nadetastic with more details: https://github.com/aws-amplify/amplify-js/issues/13096
Hi @timharsch, Wanted to check if you're still blocked on upgrading to V6 to tackle this issue. Back porting the fix from V6 to V5 would result in a breaking change.
"webpack": "^5.75.0" is a devDependency in V6. Could you please elaborate more on the blocker you're facing
Hi @timharsch, Wanted to check if you're still blocked on upgrading to V6 to tackle this issue. Back porting the fix from V6 to V5 would result in a breaking change.
"webpack": "^5.75.0" is a devDependency in V6. Could you please elaborate more on the blocker you're facing
For our app, we have other packages that would not be happy if we try to change the webpack version, which, currently is preventing us from upgrading to v6 of Amplify. Right now, my dev that needs to upgrade us to v6 has been on other critical tasks, I'm hoping to get some his time for this in next couple of weeks.
We did not try nate's test sequence unfortunately, because we are not on v6, but looking at those steps I'm not sure how they could validate that it works. What good would it do to add /code=1234 manually after the redirect? It's in the redirect process that the param is dropped isn't it? Do you have an automated test that could validate the expectations?
For our app, we have other packages that would not be happy if we try to change the webpack version, which, currently is preventing us from upgrading to v6 of Amplify. Right now, my dev that needs to upgrade us to v6 has been on other critical tasks, I'm hoping to get some his time for this in next couple of weeks. We did not try nate's test sequence unfortunately, because we are not on v6, but looking at those steps I'm not sure how they could validate that it works. What good would it do to add /code=1234 manually after the redirect? It's in the redirect process that the param is dropped isn't it? Do you have an automated test that could validate the expectations?
V5 supports a global urlListener where, when oauth is configured and the url contains code
, error
or access_token
query params it assumes it's redirected from HostedUI and proceeds to fetch tokens and credentials.
(even if there isn't an oauth in flight)
To test this flow in V5
Amplify.configure()
)code
, error
or access_token
query paramsThis reason this was designed in this manner was to support:
However V6 architecture is different, it does not contain a global urlListener and hence this issue does not exist
Before opening, please confirm:
JavaScript Framework
React
Amplify APIs
Authentication
Amplify Categories
auth
Environment information
Describe the bug
This is an odd one.
We had a callback that we needed for Xero (3rd party accountancy software) authentication which wants to send us a code on the querystring such as
/mycallbackurl/?code=123454&secret=56789
Trouble is our App would always redirect to the root url and our code to handle the callback would never fire! After some testing we found that its just the querystring name of "code" that is causing the issue. We suspect that our Cognito Authentication is being too eager and hoovering up anything with "code" in the querystring regardless of the url structure.
To work around it and prove the point I added this to the top of my index.js in the root of the App before anything else can run:
Its not pretty, but it renames the code querystring param to something else if we know its for Xero. Then in our xero callback handling code we look for the new value. This now works! But its damn weird and something we thought would catch out others. I'll include our UserProvider at the bottom so you can see how we are using it for the Authentication if it helps. The above code though is the only bit of "quirky" stuff we have in there.
Expected behavior
I'd have thought that the Cognito code would only kick in on whatever callback url it wants to you (yes this could even be the root url) rather than on everything. Seems a bit too much of a over-reach of the querystring for me.
Reproduction steps
This is hard to unpick, if you have an authed site already then you could wrap our UserProvider around it and see if you can get that running.
Then navigate to a sub folder (ie /myfolder/) and try adding
?code=1234
to the end. The site should redirect/reload at the root url and your querystring will have disappeared. Yet if you go back to the same folder but add?code2=1234
to the end it won't redirect and will instead just render and leave the querystring on the url.Code Snippet
Log output
aws-exports.js
No response
Manual configuration
No response
Additional configuration
No response
Mobile Device
No response
Mobile Operating System
No response
Mobile Browser
No response
Mobile Browser Version
No response
Additional information and screenshots
No response