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.

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

Authentication #546

Open
jpcaram opened this issue Mar 9, 2019 · 28 comments
Open

Authentication #546

jpcaram opened this issue Mar 9, 2019 · 28 comments

Comments

@jpcaram
Copy link

jpcaram commented Mar 9, 2019

I apologize if this is not the right place to ask questions. I couldn't find a discussion group or mailing list.

I recently started looking at the examples and documentation, and before I fully dive in, invest in learning it and developing larger web apps, I wanted to understand if it is fully intended for multi-user web applications.

The first thing that comes to mind is how to implement user authentication. Any simple approach?

I've seen in the documentation that there is a concept of "session", but I'm not sure how it fits with the concept of user session in a traditional web application and how you keep it secure. In web frameworks such as Flask or Bottle there is an authentication layer and also a session layer that take care of this. What would be the Flexx equivalent?

Thanks for any advise!

@almarklein
Copy link
Member

if it is fully intended for multi-user web applications.

Flexx could be used for that. If "multi" means a few, than Flexx can help connect things up (e.g. collaborative stuff). If "multi" is meant as in a more classical web server, than it's probably better to use Flexx to build the front end, but use flx.App.export() or flx.App.dump() to create static assets and serve these with your favorite web server.

In the latter case authentication is out of the scope of Flexx. In the former ... I've never had the need/time to have a proper go at it yet.

@jpcaram
Copy link
Author

jpcaram commented Mar 11, 2019

create static assets and serve these with your favorite web server.

Well, in my opinion, the power in Flexx is in the interaction between the front-end and back-end. There is no "fun" in using it just for writing the front-end and avoiding Javascript, without an interesting Python-based backend part.

Do you have any ideas on how to go about implementing authentication using the full stack solution (Tornado-based server)?

I believe that not having some authentication mechanism (or guidelines for it) is a big barrier for adoption as a web framework (as opposed to a desktop app framework, where authentication is irrelevant). After all, most web apps provide access to user data, which is always password and encryption protected.

@almarklein
Copy link
Member

I see your point, and yeah, it would be good for Flexx to have some sort of "builtin" solution for auth. Perhaps Tornado's mechanism can be leveraged?

@jpcaram
Copy link
Author

jpcaram commented Mar 17, 2019

I would be happy to help. I have experience with web apps in general, by I'm new to websockets.

I googled "tornado websockets authentication" and I found a fair amount of interesting reads. There doesn't seem to be anything standard though.

Perhaps it would be better to start with guidelines instead of going straight for a built-in solution. I usually prefer a modular approach, that way things are easier to integrate with existing code.

If we could start by discussing the architecture of Flexx, specifically the concept of "session" (I believe this is what needs to be protected), it would be ideal for me to understand what, when, and how the dialog between server and client must be secured.

@almarklein
Copy link
Member

Perhaps it would be better to start with guidelines instead of going straight for a built-in solution. I usually prefer a modular approach, that way things are easier to integrate with existing code.

I think this makes a lot of sense.

What we call a "session" in Flexx is bound to the websocket connection. But for authentication this "scope" might be too limited, as a user would need to login each time that the page is loaded. Many auth methods involve a cookie, which can be set via Flexx' session object (see set_cookie() and get_cookie()).

@jpcaram
Copy link
Author

jpcaram commented Mar 19, 2019

How about something like this: Have an authentication app (app in the sense of Flexx apps) which would set a cookie upon successful authentication and then re-direct to the second app (the intended target app). When the connection of the second app gets established, the first thing that is checked is the cookie. If it does not correspond to a valid logged-in user, it re-directs back to the login app and close the connection. If it does correspond to a valid logged-in user, we make some User object available to the code.

Of course, we would need to discuss the implementation details.

Another option is to dig into Jupyter Notebook/Lab. This is based on Tornado with websockets (you probably already knew that). And we copy from the more experienced guys. Unfortunately, I think Jupyter has just a single-user solution.

@almarklein
Copy link
Member

That sounds reasonable. It would also allow different variants of the authentication app, but the general approach can stay the same: an auth app sets a cookie, in the actual app this cookie is checked.

I agree that it could be worth looking at how Jupyter does it's authentication and see if we can use (part) of that approach.

@jpcaram
Copy link
Author

jpcaram commented Mar 29, 2019

This is where my lack of knowledge about websockets limits me: Would it be secure enough to check the cookie in the main app when the connection is first created or would we need to check it periodically? Is there a mechanism (a callback) to run code when the connection is established so that we can check the cookie? Are cookies sent by the client when a websocket connection is established?

I should do some reading, but I ask in case you already have some insight into this.

@almarklein
Copy link
Member

Good points. In theory it should be ok to accept the ws connection and check the cookie then. But my gut does not feel too good about that, because the ws connection can trigger a lot of magic. We'd have to do some sort of code review to identify potential risks. Perhaps it's better to check auth before accepting the ws ...

@jpcaram
Copy link
Author

jpcaram commented Apr 14, 2019

I created a short example with the basics: https://gist.github.com/jpcaram/696df0b2357326c4fd3965ef0d90f1c2

This should help being more concrete about the things we are talking about.

This example is written in pure user code, without any intervention in the Flexx code. There is an authentication app and a main app. The cookie is set by the auth app, and checked in init() of the main app. You are right about a lot going on before the cookie is checked. It works, but things start to get a little dirty. The first problem that I found is that the decorators for the methods will get called regardless of authentication. I used a hack for this in the code as you will see.

If the decorators are the only problem, can we just override the decorators?

What else do you think will break this approach?

@jpcaram
Copy link
Author

jpcaram commented May 13, 2019

Any feedback? I would like to continue working on this, so some suggestions as to where to look and things to try would be useful and much appreciated.

@almarklein
Copy link
Member

That's a nice example, especially since it does not even touch the Flexx code yet :)

For the decorator, I think a dummy widget is indeed the best solution (though it feels a bit awkward).

A potential risk could be that even though you're not creating the GUI for the user (instead showing "not logged in"), the code for the GUI could be present in JS. So an attacker might be able to still revive the GUI. So the current approach would feel fragile if you want to use it to shield a user from visiting certain pages.

That said, if all risk is contained in a few server-side actions, you could check the cookie there as well and you should be safe. Although ... the fact that there's an active websocket to the server (with quite a bit of complex stuff going on close to it) does leave an uncomfortable feeling :)

Ideally, I think, it would be nice to serve 403 for main if the user is not authenticated, but that'd need changes in the server code...

Does this help to get you going?

@matkuki
Copy link
Contributor

matkuki commented May 29, 2019

@jpcaram and @almarklein
Has there been any additional progress on this topic?
Is the code in the above example safe to use in a simple GUI application that runs on a local network?
It's a really great feature to have.

@jpcaram
Copy link
Author

jpcaram commented May 30, 2019

The example that I provided earlier is safe as long as you understand the limitations of it. Some we have already discussed in this thread.

I'm still working on this though, so there might be changes to the Flexx code in the near future that make the whole process more robust.

But for the time being, and specially if it is in a local network (i.e. with trusted users), it should be fine.

@matkuki
Copy link
Contributor

matkuki commented Jun 13, 2019

@jpcaram
Thanks. I got the system working, but I still have one problem.
I have two pages EntryPage and MainPage which I both serve with:

entry.serve('')
main.serve('MainPage')

When it is a fresh start and I try to directly access localhost:49190/MainPage it correctly redirects me to the EntryPage. But if I login on the EntryPage, which then redirects me to the MainPage, then close and reopen the browser, then again try to get to the MainPage directly with url localhost:49190/MainPage, it accepts it and in my console it shows that the session cookie is still valid (LoginOk from your example).
How do I reset this every time a user closes the browser, so that the credentials need to be entered every time the application is opened?
If I am missing something obvious I apologize, I'm quite inexperinced with web programming.

Thanks

@jpcaram
Copy link
Author

jpcaram commented Jun 13, 2019

@almarklein , do you mind if we discuss @matkuki 's specific issue here, even though the original purpose of this thread was for the development of the authentication features?

@matkuki , closing the browser doesn't mean anything. The cookie is still in your browser when you restart or reload the page, and that is what the server is checking. You need to add some more logic on the server side to do something like 1) allow the use of the cookie only once or 2) allow the use of the cookie for a given amount of time. In these cases you need to track the unique identity of the cookie and the time it was generated respectively.

@matkuki
Copy link
Contributor

matkuki commented Jun 13, 2019

@jpcaram
Great, thanks👍

@jpcaram
Copy link
Author

jpcaram commented Jun 13, 2019

@almarklein , before submitting any pull requests I wanted to discuss my approach.

I think that we need to call a method in the app before we call init(). For the sake of the discussion, lets call it prestart(). prestart() should be called immediately before init(), and depending on the return value of prestart(), we either run init() or do something else. Perhaps prestart() can raise an exception that it is caught in the server, before creating the session and the websocket.

This approach is much more general than just authentication and it is completely invisible to existing users until they decide to implement prestart(). Then we can build an authentication mechanism on top of it.

Your feedback is much appreciated. Thanks.

@almarklein
Copy link
Member

@jpcaram That sounds reasonable. What class would prestart() be a method of? The App class?

@jpcaram
Copy link
Author

jpcaram commented Jun 17, 2019

@almarklein It would be a method of Component, which if I'm understanding correctly it what gets wrapped with App, right?

So, the usage pattern would be:

class MyApp(PyComponent):
    
    def prestart(self):
        if not authenticated:
            raise PreStartError('Not authenticated')

    def init(self):
        // Stuff available to the authenticated user.

Then we catch PreStartError in the right place, when the app is requested by the browser, but before we send anything back (and of course, before opening the websocket). We return instead some http code indicating failure. Alternatively, the user may define some alternative action (defining overriding some method), perhaps redirecting somewhere else.

@almarklein
Copy link
Member

Ah right. This looks like a sensible approach!

@jpcaram
Copy link
Author

jpcaram commented Jun 22, 2019

@almarklein could you help me out? My implementation of the prestart mechanism is almost done. However, I'm having some strange behavior.

This is what I have done so far:
In event/_component.py:

class Component(...):
    def prestart(...):
        pass

    def __init__(...):
        # Run user-defined initialization behavior if this is an app about to
        # establish a Session.
        if 'flx_is_app' in property_values and property_values['flx_is_app'] \
                and 'flx_session' in property_values:
            self.prestart(*init_args, **property_values)
        ...

In _app.py:

class AppManager(...):
    def create_session(...):
        ...
        # Instantiate the component
        # This represents the "instance" of the App object (Component class + args)
        try:
            app(flx_session=session, flx_is_app=True)
        except PreStartError:
            del self._session_map[session.id]
            raise
        ...

And finally, in _tornadoserver.py:

class AppHandler(...):
    def _get_app(...):
        ...
        if session_id:
            ...
        else:
            try:
                session = manager.create_session(app_name, request=self.request)
                self.write(get_page(session).encode())
            except PreStartError as e:
                self.write(str(e))

In general it seems to behave as expected. I override prestart() in my PyComponent to raise a PreStartError and I get the message on the browser. But I suspect there is something that I'm not handling correctly. I get this in the console:

Exception ignored in: <object repr() failed>
Traceback (most recent call last):
  File "/home/jpcaram/cloud/flex_develop/flexx/flexx/event/_component.py", line 350, in __del__
    if not self._disposed:
AttributeError: 'Main' object has no attribute '_disposed'

I does not break anything, it continues to run normally. I tried handling the exception in the constructor, adding self._disposed = False and re-raising, but then it starts complaining about self._id. Every time I add the missing attribute, it tries to access some other one. So, it seems that some part of the program is trying to use the object created with app(flx_session=session, flx_is_app=True).

Any idea of what I'm missing? Thanks.

@almarklein
Copy link
Member

Mmm, prestart() happens too early, the component is missing some essentials :) I think it would be better to implement somewhere in app/component2.py, e.g. by overloading _comp_init_property_values() in the PyComponent class. That method is called just before init() gets called.

This got me thinking: why not simply implement the logic in init() itself? We could create a special exception class that can be raised from there, and then caught in the appropriate places.

@jpcaram
Copy link
Author

jpcaram commented Jun 25, 2019

Could you write a short summary of the sequence of events when an app is requested until it is written to the client? I'm a bit in the dark. I want to make sure there is nothing left behind once we raise the exception. I would like to run prestart() before anything happens, or that I can fully undo what has been done when we catch the exception.

It might be possible to implement the logic in init(), but I think it would be best to hide it away from most users. This way we could create an authentication mechanism in some subclass, say SecurePyComponent, that has an easy usage and the user can write init() just like before. I think it is better to keep things separate.

@almarklein
Copy link
Member

I think it is better to keep things separate.

I first thought that init() would be fine, since users could simply do super().init(). But if they forgot, the auth is gone :P Besides, I now realize that there is a better point to hook into.

Component.__init__() first does some basic initialization, before it calls _comp_init_property_values() and init(). It's only after calling init() that it sets up (event) connections with other components.

The Component._comp_init_property_values() initializes property values, but is also used as a hook in the JSComponent and PyComponent classes. They use this method because it's the best time in the initialization cycle (right before init()), and because they need the user-specified properties that get passed in as an argument.

The stuff going on in _component2.py with _comp_init_property_values()` /app/_component2.py#L352) is what makes the Py/JS interaction possible, and is a bit complex.

But that does not matter if we use _comp_init_property_values() as a hook too :)

class SecurePyComponent(PyComponent):
     def _comp_init_property_values(self, property_values):
         # ... do your thing here, raise if needed
         # Otherwise, go on as usual
         super()._comp_init_property_values(property_values)

@jpcaram
Copy link
Author

jpcaram commented Jul 2, 2019

Thanks for the explanation.

I tried your suggestion, and also running prestart() in other places. The problem now is that the destructor (__del__()) of the Component is looking for stuff that isn't there. Specifically, it is looking for things referenced in the decorators, but those thing aren't there because they were supposed to get created in init() which we intentionally did not run.

I could override __del__() and have it check some flag that is set when PreStartError is caught. I suspect that it is fine to do nothing in __del__() in such case (have it just return). Please correct me if I'm wrong.

@almarklein
Copy link
Member

Ah right. Looking at the code, I think these three attributes are unset if we don't call _comp_init_property_values of the superclass: _session and _root from BaseAppComponent and _event_listeners, __event_types_at_proxy and _has_proxy at LocalComponent.

I could override del() and have it check some flag that is set when PreStartError is caught.

I fear that not calling the appropriate dispose methods may bite us sooner or later. I think the best would be is to initialize (to None or empty list) these attributes in __init__ instead of _comp_init_property_values.

@mariohock
Copy link

Hi, I also need authentication in my app and just found this issue. It seems that this approach just died somehow close to completion. What happened? Or, perhaps more relevant: What's the state of authentication in Flexx?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants