Skip to content

Conversation

jfrconley
Copy link
Contributor

@jfrconley jfrconley commented Sep 23, 2025

Describe your changes

Add support for integrating pocketid with netbird, allowing user information to be properly synced and displayed.
The goal is to bring support inline with integrations for other self hosted idp's with this implementation cribbing heavily from the existing keycloak and zitadel integrations.

This takes advantage of the PocketID REST API to do basic user data management.

Issue ticket number and link

This should fix the issue described in #3295, populating the email and name fields in the profile from data returned by PocketID

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

netbirdio/docs#432

@CLAassistant
Copy link

CLAassistant commented Sep 23, 2025

CLA assistant check
All committers have signed the CLA.

@jfrconley
Copy link
Contributor Author

While implementing this, I did run into some confusion when looking at the zitadel and keycloak implementation. Is it intentional that updating the app metadata is optional? PocketID also does not have the direct concept of app metadata, though you could probably work something out using custom claims.

@jfrconley jfrconley marked this pull request as ready for review September 23, 2025 18:29
@jfrconley jfrconley changed the title Basic PocketID IDP integration [management] feat: Basic PocketID IDP integration Sep 23, 2025
@jfrconley
Copy link
Contributor Author

Hello @hakansa @pascal-fischer! Would it be possible to get a review?

@braginini
Copy link
Contributor

I tested this on my local self-hosted instance and it works well @jfrconley

I'm gonna look into code one more time and I think that we are good to merge

}

// TODO: read and intepret PocketID Error
return nil, fmt.Errorf("Unexpected status code: %d", resp.StatusCode)
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to be more specific in the message and mention that the error originates from PocketID

Copy link
Contributor Author

@jfrconley jfrconley Oct 16, 2025

Choose a reason for hiding this comment

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

Yeah that makes sense. I updated the messaging to specify that the unexpected status code comes from the pocket id API

@braginini braginini self-requested a review October 15, 2025 14:10
Copy link
Contributor

@braginini braginini left a comment

Choose a reason for hiding this comment

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

See the suggestions, plz

Copy link

@jfrconley jfrconley requested a review from braginini October 16, 2025 01:50
@braginini braginini merged commit bb37dc8 into netbirdio:main Oct 16, 2025
55 of 56 checks passed
@braginini
Copy link
Contributor

@jfrconley Thank you for the contribution!

@mangeld
Copy link

mangeld commented Oct 16, 2025

Turbocool, can't wait to try this. Sorry to bother you with this but @jfrconley , @braginini do anyone of you know if there's docker builds for the main branch? Or should I do my own builds to test it?

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.

4 participants