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 missing field icon_url to ConnectionOptionsOkta #153

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

adrianosela
Copy link
Contributor

🔧 Changes

Adding missing field icon_url to struct ConnectionOptionsOkta with the name LogoURL following the conventions on the other provider types' options i.e. both ConnectionOptionsOIDC and ConnectionOptionsGoogleApps have a field called LogoURL with JSON tags json:"icon_url,omitempty".

The Auth0 API already supports this field in this option -- evidence below in Testing section.

This change would be really nice because it is blocking me from enabling my customers to set icon URLs on their Okta integrations (a feature we've built on-top of Auth0 leveraging Auth0 Organizations).

📚 References

🔬 Testing

Using the HTTP post body in a file called payload.json (note that fields marked [HIDDEN] means I don't wish to share their value here):

✔ ~/go/src/github.com/adrianosela/go-auth0/management [main|…1]
20:23 $ cat payload.json
{
  "name": "demo-idp-name",
  "display_name": "demo-idp-display-name",
  "strategy": "okta",
  "show_as_button": true,
  "options": {
    "domain": [HIDDEN],
    "client_id": [HIDDEN],
    "client_secret":  [HIDDEN],
    "icon_url": "https://www.freepnglogos.com/uploads/rick-and-morty-png/rick-and-morty-morty-face-kushmastafresh-deviantart-27.png"
  }
}

The POST request:

✔ ~/go/src/github.com/adrianosela/go-auth0/management [main|…1]
20:23 $ cat payload.json | http POST https://adrianos-demo-tenant.us.auth0.com/api/v2/connections "Authorization: Bearer $AUTH0_MGMT_API_DEMO_TOKEN" | jq
{
  "id": "con_gcy2quO9uf680Xtg",
  "options": {
    "domain": [HIDDEN],
    "client_id":  [HIDDEN],
    "client_secret":  [HIDDEN],
    "icon_url": "https://www.freepnglogos.com/uploads/rick-and-morty-png/rick-and-morty-morty-face-kushmastafresh-deviantart-27.png",
    "scope": "openid profile",
    "issuer":  [HIDDEN],
    "authorization_endpoint": [HIDDEN],
    "userinfo_endpoint": [HIDDEN],
    "jwks_uri": [HIDDEN],
    "token_endpoint": [HIDDEN]
  },
  "strategy": "okta",
  "name": "demo-idp-name",
  "is_domain_connection": false,
  "show_as_button": true,
  "display_name": "demo-idp-display-name",
  "enabled_clients": [],
  "realms": [
    "demo-idp-name"
  ]
}

Getting the connection to verify the stickiness of the icon_url setting:

✔ ~/go/src/github.com/adrianosela/go-auth0/management [main|…1]
20:23 $ http https://adrianos-demo-tenant.us.auth0.com/api/v2/connections/con_gcy2quO9uf680Xtg "Authorization: Bearer $AUTH0_MGMT_API_DEMO_TOKEN" | jq
{
  "id": "con_gcy2quO9uf680Xtg",
  "options": {
    "scope": "openid profile",
    "domain": [HIDDEN],
    "issuer": [HIDDEN],
    "icon_url": "https://www.freepnglogos.com/uploads/rick-and-morty-png/rick-and-morty-morty-face-kushmastafresh-deviantart-27.png",
    "jwks_uri": [HIDDEN],
    "client_id": [HIDDEN],
    "client_secret": [HIDDEN],
    "token_endpoint": [HIDDEN],
    "userinfo_endpoint": [HIDDEN],
    "authorization_endpoint": [HIDDEN]
  },
  "strategy": "okta",
  "name": "demo-idp-name",
  "is_domain_connection": false,
  "show_as_button": true,
  "display_name": "demo-idp-display-name",
  "enabled_clients": [],
  "realms": [
    "demo-idp-name"
  ]
}

My connection in the auth0 console:
Screenshot 2023-01-25 at 8 31 19 PM

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@adrianosela adrianosela requested a review from a team as a code owner January 26, 2023 02:33
@codecov-commenter
Copy link

Codecov Report

Base: 94.89% // Head: 94.89% // No change to project coverage 👍

Coverage data is based on head (90d93ef) compared to base (61b4506).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #153   +/-   ##
=======================================
  Coverage   94.89%   94.89%           
=======================================
  Files          38       38           
  Lines        6506     6506           
=======================================
  Hits         6174     6174           
  Misses        264      264           
  Partials       68       68           
Impacted Files Coverage Δ
management/connection.go 71.35% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@sergiught sergiught left a comment

Choose a reason for hiding this comment

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

Hey @adrianosela 👋🏻

Thanks a lot for the contribution! The only thing we're missing is to generate the accessors for this new field. You can do so by simply running make generate.

I would have pushed them myself to your branch but unfortunately the checkbox to allow maintainers to push to your branch is disabled.

@adrianosela
Copy link
Contributor Author

adrianosela commented Jan 26, 2023

Hey @sergiught! Done -- thanks for picking this up so quickly!

When can I anticipate a new release to be available?

@sergiught
Copy link
Contributor

Awesome, thanks @adrianosela 👍🏻

We'll ship a new release very soon, potentially tomorrow or early next week.

@sergiught sergiught merged commit a215480 into auth0:main Jan 26, 2023
@sergiught sergiught mentioned this pull request Jan 26, 2023
6 tasks
@sergiught
Copy link
Contributor

Hey @adrianosela 👋🏻 the release with your contribution is out 🥳 -> https://github.com/auth0/go-auth0/releases/tag/v0.15.0

@sergiught
Copy link
Contributor

sergiught commented Jan 26, 2023

We're updating as well our README and adding a CONTRIBUING guidelines to make it easier to contribute to the SDK: #156, soon to be merged in. Feel free to give it a look.

@adrianosela
Copy link
Contributor Author

@sergiught awesome news! Thanks for the super-fast turnaround!

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