Skip to content

[WIP] Better auth#60

Closed
vporton wants to merge 63 commits into
apssouza22:mainfrom
vporton:feat/better-auth
Closed

[WIP] Better auth#60
vporton wants to merge 63 commits into
apssouza22:mainfrom
vporton:feat/better-auth

Conversation

@vporton

@vporton vporton commented Oct 19, 2023

Copy link
Copy Markdown
Contributor

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

@apssouza22

Copy link
Copy Markdown
Owner

@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

vporton commented Oct 21, 2023

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Owner

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

vporton commented Oct 22, 2023

Copy link
Copy Markdown
Contributor Author

@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

Copy link
Copy Markdown
Owner

@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

vporton commented Oct 27, 2023

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Owner

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

@apssouza22 apssouza22 closed this Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants