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

Feat: Add user auth #1869

Closed
wants to merge 5 commits into from
Closed

Feat: Add user auth #1869

wants to merge 5 commits into from

Conversation

iFurySt
Copy link
Collaborator

@iFurySt iFurySt commented May 18, 2024

The PR will introduce the Basic Auth for OD:

  1. Only for auth and no user data saved, because we don't have a DB for now.
  2. Use the uid and sid to enhance the session management. we only have the sid for now. so we can manage the sandbox containers like opendev_sandbox_uid_sid. I also think we can consider stopping the container rather than removing it, and the user can resume the previous session without re-init, and regular cleaning of the useless containers (but the memory/agent context needs to be stored and then recalled when resumed).
  3. Some guys have the situation to deploy the OD in the same bare machine, this PR can offer the first step to provide one instance to serve multi-users. (There was the problem of container name conflict, fix the issue where session id might be duplicated #1557 add the random str to container name, this is more like the temporary solution)
  4. Create a Share-OpenDevin for people to share their interaction trajectories #1802 needs some user info when sharing the interaction trajectories.

* Add GitHub OAuth.
* Add Google OAuth.

Signed-off-by: ifuryst <ifuryst@gmail.com>
Copy link
Collaborator

@amanape amanape left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What problem are we solving by introducing auth? We don't yet have any persistence to store or secure anything, and we aren't doing anything with their data either.

frontend/src/components/modals/auth/SigninModal.tsx Outdated Show resolved Hide resolved
frontend/src/App.tsx Show resolved Hide resolved
frontend/src/App.tsx Outdated Show resolved Hide resolved
@neubig
Copy link
Contributor

neubig commented May 18, 2024

Yeah, I have a similar question about the necessity for user auth at the moment. Currently we're developing OpenDevin as single-user software that runs locally. We definitely plan on having a hosted demo, but there are a number of ways of doing this, and depending on the method that is used there the strategy for doing auth would be different. My suggestion would be to put this on the backburner for now, and figure it out when we have a better idea of what a multi-tenant system would look like.

Signed-off-by: ifuryst <ifuryst@gmail.com>
@iFurySt
Copy link
Collaborator Author

iFurySt commented May 18, 2024

Okay, if we all believe it's unnecessary at the moment, I can stop working here for now.

@neubig
Copy link
Contributor

neubig commented May 18, 2024

That might be best for now, but thanks for the contribution! We should circle back once we have a better idea of what this will look like.

@iFurySt iFurySt closed this May 20, 2024
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.

None yet

3 participants