-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Sandcastle login #13079
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
base: main
Are you sure you want to change the base?
Sandcastle login #13079
Conversation
|
Thank you for the pull request, @jjspace! ✅ We can confirm we have a CLA on file for you. |
|
@ggetz this wont work in CI until I extend the build to provide a different client id but you should be able to test it locally. There's some code cleanup and separation that still needs to happen with the components and a list of TODOs above but the core is working. |
|
Thanks @jjspace! Do you mind providing some background on the decisions around token storage, PKCE, etc? Since Sandcastle allows running arbitrary JS code, we need to make sure we're following all best practices with regards to security. |
|
I think continuing to run serverless there may only be a limited amount of options available to us for the token. As you mention the primary concern seems to be running sandcastle code that someone else provided that can access local storage, extract your oauth token and send it somewhere for someone else to use. Depending what scopes that has it could expose a lot of your ion info. The ideal solution sounds like it would be relying on and taking advantage of cookies which are inherently limited by domain. This would probably work great for us since Another option is to limit the ability to run sandcastles loaded with a code url to make sure you "trust" it before running. This shifts the responsibility to the user to make sure they're not running code that may be malicious. However this could be a very annoying from a UX perspective. We also intentionally try to target users that are newer or non-tech savvy and may just click "run" regardless of understanding. Not really a workable solution I'm also going to look more into the security capabilities of iframes again. I know there are ways to further restrict what scripts running inside them can modify but I don't know if it's possible to prevent them from accessing localstorage for the containing application. It may not be possible with the current setup. |
Description
Initial PR implementing basic ion oauth login flow. This PR includes
+in the sidebar but working on getting the default page load hooked up)TODO:
Issue number and link
No issue
Testing plan
npm run devLoginin the top right+on the left and make sure it loads your access token and worksAuthor checklist
CONTRIBUTORS.mdCHANGES.mdwith a short summary of my change