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

Possible to use idpName instead of uuid? #66

Open
nspaul opened this issue May 11, 2023 · 2 comments
Open

Possible to use idpName instead of uuid? #66

nspaul opened this issue May 11, 2023 · 2 comments

Comments

@nspaul
Copy link

nspaul commented May 11, 2023

I am attempting to migrate from aacotroneo/laravel-saml2 to this package. In the old package, I used the idpName in the URL. When I create a tenant in this new package, it seems to want me to use a UUID in the URLs. For full context, here is my route:list output:

POST      saml2/{idpName}/acs ................................ saml2_acs › Aacotroneo\Saml2 › Saml2Controller@acs
GET|HEAD  saml2/{idpName}/login .......................... saml2_login › Aacotroneo\Saml2 › Saml2Controller@login
GET|HEAD  saml2/{idpName}/logout ....................... saml2_logout › Aacotroneo\Saml2 › Saml2Controller@logout
GET|HEAD  saml2/{idpName}/metadata ................. saml2_metadata › Aacotroneo\Saml2 › Saml2Controller@metadata
GET|HEAD  saml2/{idpName}/sls ................................ saml2_sls › Aacotroneo\Saml2 › Saml2Controller@sls
POST      saml2/{uuid}/acs ........................................ saml.acs › Slides\Saml2 › Saml2Controller@acs
GET|HEAD  saml2/{uuid}/login .................................. saml.login › Slides\Saml2 › Saml2Controller@login
GET|HEAD  saml2/{uuid}/logout ............................... saml.logout › Slides\Saml2 › Saml2Controller@logout
GET|HEAD  saml2/{uuid}/metadata ......................... saml.metadata › Slides\Saml2 › Saml2Controller@metadata
GET|HEAD  saml2/{uuid}/sls ........................................ saml.sls › Slides\Saml2 › Saml2Controller@sls

We use on-prem ADFS for our SSO, and I have all of the IDP/Relying Party stuff setup, and I just want to migrate over to this new (actively-supported) package. Practically speaking, I will only ever have 1 tenant. How can I keep the same URL scheme as before, when I was using the Aacotroneo package?

Thanks in advance!

@nicolus
Copy link

nicolus commented May 19, 2023

Hi there,

Just wanted to pitch in and say we are in the exact same situation, migrating from nirajp/laravel-saml2 (which is a fork of aacotroneo/laravel-saml2) and we'd really like to avoid asking all our tenants to update their IdP config to use a UUID, plus it's just easier for us to maintain and debug when the id in the URL is the name of the tenant.

We've only tested it very briefly, but so far it looks like updating the value in the saml2_tenants table to put any string instead of a UUID works. I don't think the code actually relies on it being an actual UUID and it just needs to be unique. If this can be confirmed, all we'd need would be to rename uuid to something like tenant_id, and add an optional --tenantId option to the saml2:create-tenant command (and keep the UUID as a default) and we should be good to go.

I'll update when I've tested this more thoroughly and maybe make a PR if there's some interest in this.

@nspaul
Copy link
Author

nspaul commented May 19, 2023

This is exactly the band-aid solution I landed on, and it is working for us so far. Your suggestions sound reasonable to me. Coming from the other angle, though, my thought was to just have the package give us the option of using either the uuid or the key in the generated route. I don't know enough to know if that would bite us down the road.

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

No branches or pull requests

2 participants