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

🔒 Generic Oauth installer #1150

Merged
merged 5 commits into from Jul 27, 2023
Merged

🔒 Generic Oauth installer #1150

merged 5 commits into from Jul 27, 2023

Conversation

awtkns
Copy link
Contributor

@awtkns awtkns commented Jul 27, 2023

  • Adds a generic interface for external apps to authenticate and integrate with our system
  • Adds a slack integration as first demo

We should probably encrypt the access tokens in the database, that can be future

@vercel
Copy link

vercel bot commented Jul 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
agent-gpt ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 27, 2023 7:17pm

@ergomake
Copy link

ergomake bot commented Jul 27, 2023

Hi 👋

Here's a preview environment 🚀

https://next-reworkd-agentgpt-1150.env.ergomake.link

Environment Summary 📑

Container Source URL
next Dockerfile https://next-reworkd-agentgpt-1150.env.ergomake.link
platform Dockerfile https://platform-reworkd-agentgpt-1150.env.ergomake.link
db Dockerfile [not exposed - internal service]
weaviate semitechnologies/weaviate:1.19.6 https://weaviate-reworkd-agentgpt-1150.env.ergomake.link

Here are your environment's logs.

For questions or comments, join Discord.

Click here to disable Ergomake.

Comment on lines +120 to +121
id String @id @default(cuid())
user_id String
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to be more specific with id? is it an account identifier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, it maps directly to the user table :)

@shahrishabh7
Copy link
Contributor

pytest is failing - ERROR - ModuleNotFoundError: No module named 'slack_sdk'

provider String
token_type String
access_token String
scope String?
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this typically work. As in, do all oauth systems typically use a string to define scope? How will we deal with scopes that require a list of strings

) -> str:
"""Install an OAuth App"""
url = await installer.install(user)
print(url)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: print

crud = mocker.Mock()

with pytest.raises(NotImplementedError):
installer_factory("asim", crud)
Copy link
Contributor

Choose a reason for hiding this comment

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

🤨

Comment on lines +105 to +109
# Settings for slack
slack_client_id: str = ""
slack_client_secret: str = ""
slack_redirect_uri: str = ""

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be in the DB now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No this is for our app, we are installing our app into other peoples slack

from sqlalchemy.ext.asyncio import AsyncSession

T = TypeVar("T", bound="BaseCrud")
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do?

@awtkns awtkns merged commit 0ef8f45 into main Jul 27, 2023
9 checks passed
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