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

Add github manifest flow #35

Merged
merged 30 commits into from
Mar 20, 2023
Merged

Add github manifest flow #35

merged 30 commits into from
Mar 20, 2023

Conversation

anvddriesch
Copy link
Contributor

@anvddriesch anvddriesch self-assigned this Mar 9, 2023
Team: g.Team,
}
}
func (g *Github) GetCredentialsForAuthenticatedApp(config provider.AppConfig) (map[string]string, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this function is called to create credentials for dex-operator

@@ -37,12 +38,6 @@ type Config struct {
ClientSecret string
}

type config struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Somehow this deletion appeared during merging. config was actually just renamed to Config.

func (a *Azure) DeleteSecret(ctx context.Context, secretID *string, appID string) error {
requestBody := removepassword.NewRemovePasswordPostRequestBody()
func (a *Azure) DeleteSecret(ctx context.Context, secretID *uuid.UUID, appID string) error {
requestBody := applications.NewItemRemovePasswordPostRequestBody()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nothing changed here, functions were just renamed upstream

return fmt.Sprintf(`client-id: %s
client-secret: %s
tenant-id: %s`, secret.ClientId, secret.ClientSecret, a.TenantID), nil
return map[string]string{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We return a map with the credentials. The string returned earlier was just so it could be printed instantly

team: %s
app-id: %v
private-key: %s`, c.ClientID, c.ClientSecret, c.Organization, c.Team, c.AppID, c.PrivateKey), nil
func (g *Github) CreateApp(config provider.AppConfig) (*githubclient.AppConfig, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function calls the manifest flow to create a github app

}
})
// Retrieves the app code after redirecting back to the local server
mux.HandleFunc("/retrieve", func(w http.ResponseWriter, r *http.Request) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After the app manifest was submitted we get redirected to /retrieve which automatically retrieves the code we need to complete the manifest flow and creates the app with it. The app data is saved.

http.Redirect(w, r, "/complete", http.StatusFound)
})
// The flow is completed, show the user some sort of success and cancel the context
mux.HandleFunc("/complete", func(w http.ResponseWriter, r *http.Request) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After retrieving the app data we get redirected to /complete which just shows the user a success template and closes the context

return err
}

func newState() (string, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We create a unique state here that is returned from github when retrieving the code.

"strings"
)

type Manifest struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the manifest represents the github app

}

// This returns a comma seperated list of callback URLs for github applications
func getGithubRedirectURLs(domains []string) string {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

dex-operator can not create github apps itself (github api does not allow it). However, since it does not need elevated access because of that, we can use the authenticated app itself for access via dex. For this to work we need to add the redirect URLs for dex instances we want to manage to the authenticated app.

@anvddriesch anvddriesch marked this pull request as ready for review March 15, 2023 09:05
@anvddriesch anvddriesch requested a review from a team March 15, 2023 09:05
Copy link
Contributor

@mogottsch mogottsch left a comment

Choose a reason for hiding this comment

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

LGTM, but is there a way I could test this?

pkg/idp/provider/github/manifest/flow.go Show resolved Hide resolved
@anvddriesch
Copy link
Contributor Author

Testing it right now is a little bit annoying. Essentially you need to provide github credentials based on a test-app in github as input files and then execute Setup. I can show you how to do that.
However, if you can wait a little bit longer I'm actually working on importing it into an opsctl command and adding some credentials to config for test-installations. That would make testing a lot easier.

@anvddriesch anvddriesch changed the title WIP add github manifest flow Add github manifest flow Mar 15, 2023
@mogottsch
Copy link
Contributor

However, if you can wait a little bit longer I'm actually working on importing it into an opsctl command and adding some credentials to config for test-installations. That would make testing a lot easier

Sounds good.

Base automatically changed from credential-management to main March 20, 2023 10:54
@anvddriesch anvddriesch merged commit 1beed37 into main Mar 20, 2023
@anvddriesch anvddriesch deleted the github-credentials branch March 20, 2023 11:32
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.

3 participants