jeremyschulman / netbox-plugin-auth-saml2

Netbox plugin for SSO using SAML2
119 stars 21 forks source link

treat missing groups as an empty list #40

Closed jumanjiman closed 2 years ago

jumanjiman commented 3 years ago

Comments in code, but compare...

User who does not have any groups (expect empty groups claim, but get missing claim):

<AttributeStatement>
    <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress">
        <AttributeValue>John.Doe@example.com</AttributeValue>
    </Attribute>
    <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname">
        <AttributeValue>John</AttributeValue>
    </Attribute>
    <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname">
        <AttributeValue>Doe</AttributeValue>
    </Attribute>
</AttributeStatement>

User who does have groups:

<AttributeStatement>
    <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress">
        <AttributeValue>Jane.Doe@example.com</AttributeValue>
    </Attribute>
    <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname">
        <AttributeValue>Jane</AttributeValue>
    </Attribute>
    <Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname">
        <AttributeValue>Doe</AttributeValue>
    </Attribute>
    <Attribute Name="http://schemas.microsoft.com/ws/2008/06/identity/claims/groups">
        <AttributeValue>role-Netbox-Admin</AttributeValue>
    </Attribute>
</AttributeStatement>
jumanjiman commented 3 years ago

Hi @jeremyschulman

TYVM for this project. I've learned a great deal with it!

I have tested this PR with netbox in our internal network, and it does what it claims to do. Specifically, a user with groups can log in and get permissions to do stuff, but a user without groups logs in but has zero permissions and cannot even view stuff. It thus avoids a traceback in the code when the IdP omits the groups attr.

LMK if I've made some conceptual error.

TY, -paul

jeremyschulman commented 3 years ago

Hi @jumanjiman - my apologies for being MIA responding. I've reviewed the PR and it appears to be a good update. I would like to dev-test this in my environment just to do the "measure twice, cut once" thing. Really appreciate your time and contribution.

jeremyschulman commented 2 years ago

@jumanjiman - I have not forgotten :-) I am planning to test validate this next week after the Labor Day holiday.

jumanjiman commented 2 years ago

Thanks for the update, and have a wonderful weekend!

jumanjiman commented 2 years ago

@jeremyschulman gentle bump

jeremyschulman commented 2 years ago

@jumanjiman - yikes, my apologies! I'll get this done today.

jeremyschulman commented 2 years ago

deployed to PyPi v2.4. Thank you!

jumanjiman commented 2 years ago

Thank you!