Skip to content

Consider switching sample repo to 'Authorization Code with PKCE' flow #24

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

Closed
jeroenheijmans opened this issue Aug 11, 2019 · 4 comments · Fixed by #36
Closed

Consider switching sample repo to 'Authorization Code with PKCE' flow #24

jeroenheijmans opened this issue Aug 11, 2019 · 4 comments · Fixed by #36
Labels
enhancement Request for improvement

Comments

@jeroenheijmans
Copy link
Owner

The relevant working groups have updated the advice for single page applications (such as this one, which uses Angular) around the recommended flow. Latest recommendation seems to be that SPA's should switch from "Implicit Flow" to the "Authorization Code with PKCE" flow.

After several different feature requests it was recently released in version 8.

I'm not sure how exactly yet, but I would like my sample repository to somehow support showcasing Code+PKCE flow. However, many people will still want to be using Implicit flow, and it seems awkward to support both.

Some ideas how this repo could support both:

  1. Have both options in master and provide a way to toggle between the two.
  2. Have Code+PKCE as the 'primary' option in master, and refer to an older commit if folks want to see Implicit Flow in action.
  3. Have a branch for Code+PKCE for now, and leave master at Implicit (perhaps with a readme update pointing to the branch). Then later switch them around (effectively going back to option 2).

I think option 3 will have to do for the moment, as we'll need a place to start anyways.

@jeroenheijmans jeroenheijmans changed the title Consider switching to 'Authorization Code with PKCE' flow Consider switching sample repo to 'Authorization Code with PKCE' flow Aug 11, 2019
@jeroenheijmans
Copy link
Owner Author

There's now a branch for this support following option 3. I'm running into some issues with silent refreshes via iframe though, and will open an issue about that in the library repository.

@jeroenheijmans
Copy link
Owner Author

I've updated the branch so that it:

I'm not yet ready to merge it to become the main example, as I think silent refresh via iframes is a must have for applications, to offer a nice UX.

@jeroenheijmans
Copy link
Owner Author

This issue is blocked by #34

@jeroenheijmans
Copy link
Owner Author

jeroenheijmans commented Mar 29, 2020

🎉 🎉 🎉 It is now finally working!

But... it required a bit of a heavy-handed workaround in commit aa3ea01 with the core thing repeated here:

<!-- silent-refresh.html for Code Flow -->
<script>
  console.log("The silent-refresh.html file was loaded and now posting to the parent.");
  const fakeHashFragment = location.search.replace(/^\?/, "#");
  parent.postMessage(fakeHashFragment, location.origin);
</script>

I'm not sure yet if I'm happy with promoting this workaround to the main example just yet. It seemed to work just perfectly, but I feel it requires a hack that should be handled by the library. I will see if I can convince the community of the same in the related issue there.

Wow, I now see that the Silent Refresh documentation was rewritten lately, promoting what I considered a workaround as a proper solution (with some added tweaks too).

I'm not sure if I like the approach, but it seems the way forward, so will just roll with it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Request for improvement
Projects
None yet
1 participant