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: allow using only project API keys #384

Merged
merged 5 commits into from
Oct 9, 2024
Merged

Conversation

zepatrik
Copy link
Member

Prior to this change, users needed to supply a workspace API key just so the CLI was able to resolve a slug to an ID and vice versa.
We now allow project API keys to do this lookup on their own.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure, but this was just running the same test as test-e2e/test-proxy?

Comment on lines +208 to +224
if h.workspaceAPIKey != nil {
if h.workspaceOverride != nil {
workspace = *h.workspaceOverride
} else if ws, ok := os.LookupEnv(WorkspaceKey); ok {
workspace = ws
} else {
if config.SelectedWorkspace != uuid.Nil {
workspace = config.SelectedWorkspace.String()
}
return errors.New("workspace API key is set but workspace flag is also set, please remove one")
}
workspace = strings.TrimSpace(workspace)

if id, err := uuid.FromString(workspace); err == nil {
h.workspaceOverride = pointerx.Ptr(id.String())
} else if workspace != "" {
ws, err := h.findWorkspace(ctx, workspace)
ws, err := h.ListWorkspaces(ctx)
if err != nil {
return err
}
if len(ws) > 0 {
h.workspaceID, err = uuid.FromString(ws[0].Id)
if err != nil {
return nil, err
}
if ws != nil {
h.workspaceOverride = pointerx.Ptr(ws.Id)
return fmt.Errorf("unable to parse workspace ID from response: %w", err)
}
return nil
}
return errors.New("workspace API key is set but no workspaces were found")
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the new stuff for workspace API keys.

Comment on lines +259 to 278
if h.projectAPIKey != nil {
if h.projectOverride != nil {
project = *h.projectOverride
} else if pj, ok := os.LookupEnv(ProjectKey); ok {
project = pj
} else if config.SelectedProject != uuid.Nil {
project = config.SelectedProject.String()
return errors.New("project API key is set but project flag is also set, please remove one")
}
project = strings.TrimSpace(project)

if id, err := uuid.FromString(project); err == nil {
h.projectOverride = pointerx.Ptr(id.String())
} else if project != "" {
pj, err := h.findProject(ctx, project, h.workspaceOverride)
if h.workspaceID != uuid.Nil {
return errors.New("project API key is set but workspace is also set, please remove one")
}
pjs, err := h.ListProjects(ctx, nil)
if err != nil {
return err
}
if len(pjs) > 0 {
h.projectID, err = uuid.FromString(pjs[0].Id)
if err != nil {
return nil, err
}
if pj != nil {
h.projectOverride = pointerx.Ptr(pj.Id)
return fmt.Errorf("unable to parse project ID from response: %w", err)
}
return nil
}
return errors.New("project API key is set but no projects were found")
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the new stuff for project API keys.

@@ -288,43 +317,72 @@ func TestCommandHelper(t *testing.T) {

// check that the key works
ctxWithKey := client.ContextWithOptions(ctx,
client.WithWorkspaceAPIKey(sessionToken), // TODO this key should not be required, currently it is though to look up the slug
Copy link
Member Author

Choose a reason for hiding this comment

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

Finally 🎉

Comment on lines -128 to -135
// TODO: we probably don't want to support this in the future, but it requires to be authenticated
slug := os.Getenv("ORY_PROJECT_SLUG")
if slug == "" {
project, err := h.GetSelectedProject(ctx)
if err != nil {
return err
}
slug = project.Slug
Copy link
Member Author

Choose a reason for hiding this comment

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

🎉

Copy link
Member

Choose a reason for hiding this comment

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

Do we now always require an api key to be able to use the proxy and tunnel command?

Copy link
Member

@aeneasr aeneasr Oct 3, 2024

Choose a reason for hiding this comment

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

I'm asking because in many of our quickstarts we use this method for set up: https://www.ory.sh/docs/getting-started/integrate-auth/go#test-your-application

I just tried it locally on my system and it triggers an auth screen when I run ory proxy http://localhost:3000. Using -q doesn't work: can not sign in or sign up when flag --quiet is set.

This used to work just fine without auth (social sign in however did not work!) but now requires auth always.

I think the question is if we want to re-add anonymous proxy/tunnel use. What I like about this approach here is that everything just works when you're authed.

Copy link
Member Author

@zepatrik zepatrik Oct 8, 2024

Choose a reason for hiding this comment

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

The problem with anonymous use is that depending on the use-case we need either the project slug or the project ID. IMO it is confusing when you sometimes have the option to provider either, and other times need specifically one. I assumed that the added benefit of less options and more "out-of-the-box" outweighs the need to sign-in (which you probably want to do anyway). We also support better auth for non-interactive environments now, so everything should be a lot smoother with this approach.

Copy link
Member

Choose a reason for hiding this comment

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

OK

@aeneasr aeneasr merged commit 0a752c3 into master Oct 9, 2024
15 checks passed
@aeneasr aeneasr deleted the feat/simplify-api-key-usage branch October 9, 2024 18:36
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