Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Generic authentication/authorization components #2126

Closed
skabashnyuk opened this issue Jun 19, 2018 · 5 comments
Closed

Generic authentication/authorization components #2126

skabashnyuk opened this issue Jun 19, 2018 · 5 comments

Comments

@skabashnyuk
Copy link

The goal of this issues is to clarify/implements parts of Theia that can be reused by vendors if they want to make Theia multi-user.
These components should not depend (or at least it should be possible to bind different implementation) on concrete authentification way or identity management system.

  • Make it possible to add https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization header to each request made from client side to server side. It can be some kind of factory that can have different implementations. Note: some vendors may want to use cookies or query parameter as a placeholder for identification token. In my opinion, explicitly defined token as a header parameter has the best balance between security and easiness of implementation. Because in cookie case we need to make sure that there is CSRF protection mechanism enabled. Which is usually implemented by explicitly adding extra protected token as header parameter.
  • Keep identity(token) during request execution as a thread local variable on the server side. This requirement we need to be able to identify a user on each state of request execution. For instance, if during request we need to make subsequent request we should be able to put identity token to authorization header to be able to identify a user on next call.
@svenefftinge
Copy link
Contributor

+1

some questions/comments

Because in cookie case we need to make sure that there is CSRF protection mechanism enabled. Which is usually implemented by explicitly adding extra protected token as header parameter.

I'm not a security expert but aren't HttpOnly cookies more secure than a token that is explicitly added?

Keep identity(token) during request execution as a thread local variable on the server side.

There is no such thing in node (it is single threaded by design). Would be good if we can make it work for you by just providing a generic hook, rather than managing state.

@skabashnyuk
Copy link
Author

I'm not a security expert but aren't HttpOnly cookies more secure than a token that is explicitly added?

HttpOnly flag solves a bit different problem. The issue is in cookies nature. They are sent with each request automatically.
If I try to explain "CSRF" on fingers, it is like: Attacker can create some HTML page (a forum for instance) where he can embed some form

 <body onload="document.forms[0].submit()">
   <form action="http://netbank.com/transfer.do" method="POST">
     <input type="hidden" name="acct" value="AttackerA"/>
     <input type="hidden" name="amount" value="$100"/>
     <input type="submit" value="View my pictures!"/>
   </form>
 </body>

and send it to a victim. Victim's browser will send all cookies it have, including HttpOnly to http://netbank.com/transfer.do.
And if a user was authenticated on http://netbank.com/ it will include authentication cookies as well. On a server side, it would be hard to differentiate request from Theia ide or from attacker site.

There is no such thing in node (it is single threaded by design). Would be good if we can make it work for you by just providing a generic hook, rather than managing state.

I can rephrase it to such statement: we should be able to have a way to identify a user who made initial and all subsequent request.

@slemeur slemeur mentioned this issue Jun 20, 2018
Closed
20 tasks
@skabashnyuk skabashnyuk mentioned this issue Jun 20, 2018
Closed
18 tasks
@akosyakov
Copy link
Member

You can start by looking into the express middleware. There is support for CSRF protection: https://github.com/expressjs/csurf.

An extension can configure the middleware via BackendApplicationContribution.configure hook: https://github.com/theia-ide/theia/blob/aa7754c9a5a17a35e3b103514817f784dbbb648d/packages/core/src/node/backend-application.ts#L22

@skabashnyuk
Copy link
Author

Small status update.
We (@ashumilova @evidolob @vitaliy-guliy @mshaposhnik @sleshchenko) slowly moving towards in our proof of concept of "Authenticated Che + Theia".
What we found in our study that initial plan by adding "authorization" header to each Theia's request
may be too unnatural, overengineered operation. Instead, we looked on cookie option and noticed
that Theia is not doing POST requests(at least @evidolob didn't found them) that is why CSRF may be not a problem.

I would like to ask to keep this issue open until we finalize our POC. I hope it will be soon.

@akosyakov akosyakov mentioned this issue Jul 16, 2018
Closed
@skabashnyuk
Copy link
Author

We completed our POC. We also use cookies to implement authentication. The single cons of this solution are that Theia should not use (afaik it's not using for now) POST method with *-form-* content type. Otherwise this it may be vulnerable for a CSRF attack.

@vince-fugnitto vince-fugnitto mentioned this issue Feb 7, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@svenefftinge @skabashnyuk @akosyakov