Questions about security, CSP, localstorage

[adrian-moisa]

I have implemented flutter_web_auth_2 in my project. Now I have some questions. What I managed to do so far. I have a KC client to which I connect using state and pkce params. The initial login is not silent, any page reload does a silent login and so do the refresh token calls. Now the thing is that I started with the default setting for web which is to open links in _blank. I see that auth.html is using iframe messages when opened in _blank which improves the security of the setup since no tokens are stored in localstorage.

Now my question is the following: I tried to switch to _self for UX reasons. What is happening is auth.html fallbacks to transfering the tokens after redirect via localstorage. Here I have a couple of questions: Aren't tokens in local storage unsafe? I see that switchign back to _blank no longer produces tokens that linger around in the store. + I don't really understand what aren't they deleted right after the initial authorise post redirect.

I also looked into implementing CSP tags. I see that flutter requires quite a few liberties, which greatly diminishes the XSS protection you can gain from CSP. Maybe I can use nonce or potentially generate hashes for every build. Otherwise the tokens could be exposed to malicious actors.

Anyway, I'm dumping a lot of info here. Bottom line, what is your recommendation? Stick with _blank and nuke the UX or work harder to get _self more secure by deleting the tokens and implementing CSP via nonce and hash?

Edit: What about rendering an iframe in a modal on top of the flutter app. That would keep the iframe message mechanism intact. Also the UX would look seamless.

Edit2: Clickjacking - via https://datatracker.ietf.org/doc/html/rfc6749#section-10.13

In a clickjacking attack, an attacker registers a legitimate client
and then constructs a malicious site in which it loads the
authorization server's authorization endpoint web page in a
transparent iframe overlaid on top of a set of dummy buttons, which
are carefully constructed to be placed directly under important
buttons on the authorization page. When an end-user clicks a
misleading visible button, the end-user is actually clicking an
invisible button on the authorization page (such as an "Authorize"
button). This allows an attacker to trick a resource owner into
granting its client access without the end-user's knowledge.

To prevent this form of attack, native applications SHOULD use
external browsers instead of embedding browsers within the
application when requesting end-user authorization. For most newer
browsers, avoidance of iframes can be enforced by the authorization
server using the (non-standard) "x-frame-options" header. This
header can have two values, "deny" and "sameorigin", which will block
any framing, or framing by sites with a different origin,
respectively. For older browsers, JavaScript frame-busting
techniques can be used but may not be effective in all browsers.

[ThexXTURBOXx]

Now my question is the following: I tried to switch to _self for UX reasons. What is happening is auth.html fallbacks to transfering the tokens after redirect via localstorage.
I see that switchign back to _blank no longer produces tokens that linger around in the store.

The local storage approach is chosen when the other approaches would fail - it acts as a workaround, basically.
In many modern browser, window.opener is set to null when opening new tabs in specific ways to prevent tabnabbing attacks.
And the same way window.parent might be reset to null.
If both are set to null (or for some reason window.parent is invalid), authentication is using postMessage is not possible anymore.
flutter_web_auth_2 was not able to finish the authentication in these cases and users complained about it, which is why this solution was added at some point.
That's the whole story behind it - with #58 being the PR that added this fallback approach.

Aren't tokens in local storage unsafe?

In a nutshell: Not inherently - it depends on the use case, though. If your website is susceptible to XSS, yes, it is unsafe.
However, then there are a ton of additional issues to be aware about, so the local storage is really the last thing that would come to my mind.

I don't really understand what aren't they deleted right after the initial authorise post redirect.

They should be removed after successful authentication as far as I am concerned:
https://github.com/ThexXTURBOXx/flutter_web_auth_2/blob/master/flutter_web_auth_2/lib/src/web.dart#L114

Anyway, I'm dumping a lot of info here. Bottom line, what is your recommendation? Stick with _blank and nuke the UX or work harder to get _self more secure by deleting the tokens and implementing CSP via nonce and hash?

Mostly, I like going the KISS way - hence I would just diregard the UX. Maybe, I or someone else finds another way to fallback on when authenticating users on Web. Then, anyone using this package can benefit from smoother authentication procedures without needing to live with some compromise or needing to implement "unnecessary" (wrong word, but I think you get the gist) workarounds.

What about rendering an iframe in a modal on top of the flutter app. That would keep the iframe message mechanism intact. Also the UX would look seamless.

Also possible! Now that I think of it - I could even enable another parameter in FlutterWebAuth2Options where one could pass custom arguments for the iframe approach. Would that help your use case, maybe? This way, you could instruct flutter_web_auth_2 to use the "invisible iframe approach", but also instruct it to make the iframe visible.

[adrian-moisa]

For sure the visible iframe would be nice, still it's unclear for me how secure would be that option. GPT does not raise any alarms but I found some contradicting advice on the web. For now I'll keep _blank, but for sure I'd like to use _self without localstorage. And yes, according to the source code they should get deleted, but somehow they don't. I'll try reviewing later to see what goes wrong in my case. All I have added on the auth.html was a browser location update to return back to the root and to finish the login process. Afaik the authenticate() is called again and I clearly see it has delete localstorage instructions. I'll try to place some breakpoints, see what happens.