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 Osso as open source SAML provider #928

Closed
wants to merge 13 commits into from

Conversation

sbauch
Copy link
Contributor

@sbauch sbauch commented Dec 7, 2020

Per #311 this adds a Provider Osso - we're like an open source Auth0, but exclusively for SAML 2.0.

Osso provides a UI, persistence and documentation for configuring SAML identity providers, and wraps a SAML auth process inside in Oauth 2.0 auth code grant flow, which allows us to easily hook in to projects like this that implement Oauth. You can check out a demo Osso instance at https://demo.ossoapp.com and generally learn more about osso at https://ossoapp.com

In order to show how this all works, I've set up the following:

• a staging instance of Osso

this includes some work to add a "hosted login page" like Auth0 has

• a next app - https://osso-next-auth-mcwe2rmtw.vercel.app/

this is based on this project's example, but includes a module transpilation plugin to build a version of this package with support for Osso as a provider. it uses the review Osso app above via ENV vars for OSSO_DOMAIN, OSSO_ID and OSSO_SECRET. repo: https://github.com/enterprise-oss/osso-next-auth-example

• a fake Identity Provider

It takes a bit of work to configure an IDP for SAML login, so we've developed a little ruby app that acts as a fake IDP. it takes a SAMLRequest, allows a user to "log in" using any values, and crafts a SAMLResponse, sending the user back to osso and eventually the next app on vercel. repo: https://github.com/enterprise-oss/sinatra-ruby-idp

So you can test this end to end:

  1. visit https://osso-next-auth-dw7gmmt27.vercel.app/ and sign in with SAML SSO
  2. enter [email protected] on the next screen (this is our "hosted login page" similar to the Auth0 example)
  3. enter any values for email and password on our fake idp
  4. you'll be redirected back to the vercel app, and the value you entered for email on step 3 should be your "signed in as"

I'll leave some code specific comments inline.

Thanks gang, and apologies again for my nonsense with the canary branch etc. Would be really excited to get support for Osso into such a great project!

@vercel
Copy link

vercel bot commented Dec 7, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextauthjs/next-auth/o6j1ymwq2
✅ Preview: https://next-auth-git-fork-enterprise-oss-enterprise-oss-osso.nextauthjs.vercel.app

@balazsorban44 balazsorban44 changed the base branch from main to canary December 7, 2020 23:06
@balazsorban44
Copy link
Member

The intention IS to use canary as your base. This we can merge things faster and get them out to npm with a canary tag, while testing and docs will be polished before it is pushed to main and released as latest on npm.

@sbauch
Copy link
Contributor Author

sbauch commented Dec 7, 2020

thanks... wasn't clear in the contribution guidelines that was the intention

@balazsorban44
Copy link
Member

Sorry about that! We are currently transitioning to a new structure we started a few days ago. Hopefully, this will allow a faster release cycle for those who don't mind some bugs here and there. Once the details are set, we will probably update the CONTRIBUTING document.

@sbauch
Copy link
Contributor Author

sbauch commented Dec 7, 2020

no problem! sorry bout it all, hate to create more work for maintainers. its a clever approach now that i understand a bit!

anyhow, it feels like too big a change to find a way to add another query param to my authorize url based on user input? so we'll get cracking on more of a hosted sign in flow so I can at least demo this in action. It's a bit tough because in order to do a full SAML login you'll need an Osso instance (we have one at demo.ossoapp.com) and to have configured a SAML provider (Okta has a good developer account offering) in it, and assigned users, etc.

@vercel vercel bot temporarily deployed to Preview December 8, 2020 04:11 Inactive
@vercel vercel bot temporarily deployed to Preview December 8, 2020 04:16 Inactive
@vercel vercel bot temporarily deployed to Preview December 9, 2020 23:19 Inactive
Comment on lines 12 to 15
return {
id: profile.id,
name: profile.name,
email: profile.email
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 currently offer a minimal profile, but could envision adding more attributes as needed. can we ...spread the profile object? are any of these required?

Copy link
Member

Choose a reason for hiding this comment

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

Great question! The docs on this could use improving.

To work with a database, a id is required from a profile, it is stored as accountId, but name and email are optional as neither are guaranteed from a provider.

From memory:

  • If using a database then the name and email will be used when creating a new user, but if the account exists and is associated with a user, then it will use the id to look up the account, then from there look up the user associated with that account and return them for the user info when getSession() is called.
  • If using JSON Web Tokens without a database, then it doesn't matter what these values are, but if blank they will be blank in the JSON Web Token / Session.
  • The full profile returned from a provider is exposed in various callbacks. It is used here for very limited purposes. Returning extra fields here has no impact.

@sbauch sbauch marked this pull request as ready for review December 9, 2020 23:37
@vercel vercel bot temporarily deployed to Preview December 18, 2020 18:07 Inactive
@vercel vercel bot temporarily deployed to Preview December 23, 2020 01:11 Inactive
@sbauch
Copy link
Contributor Author

sbauch commented Dec 23, 2020

Our vercel teams trial expired, so the demo app for this is now at https://nextjs-demo.ossoapp.com/

lmk what i can do to push this along! more than happy to write docs etc, not sure what the expectation is for a canary merge

🙏

@vercel vercel bot temporarily deployed to Preview January 4, 2021 19:24 Inactive
@vercel vercel bot temporarily deployed to Preview January 8, 2021 21:09 Inactive
@iaincollins
Copy link
Member

@sbauch I just wanted to say have checked out the demo site, thanks for setting that up. Osso looks really cool!

Having a way to integrate with SAML auth directly would be a lot of work for something like NextAuth.js, so having an solution like this is wonderful, especially an open source product.

It would be lovely to chat sometime!

@sbauch
Copy link
Contributor Author

sbauch commented Feb 2, 2021

thanks @iaincollins! glad to hear you think this could be a good addition - it's admittedly a little weird to add support for a provider like this where the library users have to deploy or pay for an Osso instance, but since you already support a provider like Auth0 it seems reasonable to us.

I'm planning on responding to feedback here and writing some docs this week, but would be happy to connect. to avoid putting my email on github, maybe you could drop a note in our chat widget at ossoapp.com and we can take it offline from there?

@vercel vercel bot temporarily deployed to Preview February 9, 2021 13:51 Inactive
@github-actions github-actions bot added the docs Relates to documentation label Feb 9, 2021
profile: (profile) => {
return {
id: profile.id,
name: profile.name || profile.email,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

SAML spec is a little odd wrt NAMEID format things. Osso's docs don't instruct IDP admins to include a "name" attribute, but we do sometimes get them back.

I'm thinking here that if the database or user model depends on there being a name, we should at least fall back to email, which is guaranteed to be available.

@sbauch
Copy link
Contributor Author

sbauch commented Feb 9, 2021

Added some docs and got up to date here 📖 👩‍⚕️ 👨‍⚕️

@vercel vercel bot temporarily deployed to Preview February 9, 2021 13:59 Inactive
@vercel vercel bot temporarily deployed to Preview February 9, 2021 21:46 Inactive
@sbauch
Copy link
Contributor Author

sbauch commented Feb 18, 2021

added docs for this to our docs site (borrowing a bit from your docs) - https://ossoapp.com/docs/consume/nextjs

now that the big release and security patch is out, chances we could take another look here?

@sbauch sbauch requested a review from iaincollins March 1, 2021 15:27
@balazsorban44
Copy link
Member

Hi there, and thanks for your patience on this! I think we are ready to approve this, only one thing left... I am going to close this PR, because we changed our release strategy, and this PR should be opened against main instead. I could ask you to simply rebase, but we rely strictly on the commits to generate release notes, so after my findings, the easiest thing is just to re-open a PR against another branch. I am the one who asked you to change to canary in the first place, so terribly sorry for that! 🙈 If you don't wish to set up a new PR, I can happily copy your changes and make you a co-author in a new PR as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Relates to documentation providers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants