apssouza22 / chatflow

Leveraging LLM to build Conversational UIs
BSD 2-Clause "Simplified" License
120 stars 26 forks source link

[WIP] Better auth #60

Closed vporton closed 11 months ago

vporton commented 1 year ago

This is a work in progress to solve issue #10. This PR is dependent on PR #56.

vporton commented 1 year ago

@apssouza22 I have a question for you:

        this.sessionData = {
            token: params["token"] || "",
            user: params["user"] || "",
            app: params["app"] || "chat",
            isPluginMode: params["pluginMode"]=="true" || false,
            appColor: params["appColor"] || "black",
            sessionId:  localStorage.getItem("sessionId"),
        }

app is stored in the session. Does it mean that the UI is centered on one app at a time? If yes, why so? AI is able to keep in memory several apps simultaneously. For me it seems a wrong design decision to work with just one app. Please, explain.

apssouza22 commented 1 year ago

@apssouza22 I have a question for you:

        this.sessionData = {
            token: params["token"] || "",
            user: params["user"] || "",
            app: params["app"] || "chat",
            isPluginMode: params["pluginMode"]=="true" || false,
            appColor: params["appColor"] || "black",
            sessionId:  localStorage.getItem("sessionId"),
        }

app is stored in the session. Does it mean that the UI is centered on one app at a time? If yes, why so? AI is able to keep in memory several apps simultaneously. For me it seems a wrong design decision to work with just one app. Please, explain.

@vporton apps here is chatflow app. We can not run more than one at the time

vporton commented 1 year ago

apps here is chatflow app. We can not run more than one at the time

@apssouza22 A chatflow app is any API that user documented, right? Or what is it?

Why can't we run more than one at a time?

apssouza22 commented 1 year ago

Yes, but os docs will be linked to one app. It is just one way of organizing things.

One user can have multiple apps and one app can have multiple docs

vporton commented 1 year ago

@apssouza22 What are "os docs"?

You didn't answer why we need to store the app in session data, not instead decide the app by the AI at each step (not the entire session) separately.

apssouza22 commented 1 year ago

@vporton because u need to search only docs for the the given app.

Don't make a big deal of it. It is just app id in the session to tell the backend where to retrieve docs from

vporton commented 1 year ago

A good behavior of the app is to expire authentication after a certain time interval after the last operation that the user did.

Because expiration time is stored on the client side, to implement this delayed expiration, we need to re-download JWT token on each request (not only on login/signup requests).

I propose either:

  1. to download the token in a header like X-Auth-Token on every request (a little inefficient because needs to create and transfer JWT token on every request)
  2. move expiration to server-side (remove exp from the token and let the server decide).

@apssouza22 I need your principal agreement for either 1 or 2. I prefer 2.

apssouza22 commented 11 months ago

A good behavior of the app is to expire authentication after a certain time interval after the last operation that the user did.

Because expiration time is stored on the client side, to implement this delayed expiration, we need to re-download JWT token on each request (not only on login/signup requests).

I propose either:

  1. to download the token in a header like X-Auth-Token on every request (a little inefficient because needs to create and transfer JWT token on every request)
  2. move expiration to server-side (remove exp from the token and let the server decide).

@apssouza22 I need your principal agreement for either 1 or 2. I prefer 2.

Let's stick with the standard. If the token is expired the user need to loging again. Indeed we are not redirecting the user to login again if the token is expired. If you want to address that, it should be a simple PR in makeRequest call to look for response code 401