search

flipboxfactory / saml-sp

SAML Service Provider (SP) Plugin for Craft CMS
https://saml-sp.flipboxfactory.com/
Other
19 stars 5 forks source link

Issue importing XML from ADFS (Metadata file too large for db column) #48

Closed syska closed 4 years ago

syska commented 4 years ago

Having issues importing XML Metadata from ADFS ...

Trying to validate with https://www.samltool.com/validate_xml.php gives some pointers:

Line: 25 | Column: 0 --> Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor', attribute '{http://www.w3.org/2001/XMLSchema-instance}type': The QName value '{http://docs.oasis-open.org/wsfed/federation/200706}ApplicationServiceType' of the xsi:type attribute does not resolve to a type definition.

Line: 25 | Column: 0 --> Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor': The type definition is abstract.

Line: 383 | Column: 0 --> Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor', attribute '{http://www.w3.org/2001/XMLSchema-instance}type': The QName value '{http://docs.oasis-open.org/wsfed/federation/200706}SecurityTokenServiceType' of the xsi:type attribute does not resolve to a type definition.

Line: 383 | Column: 0 --> Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor': The type definition is abstract.

Removing all RoleDescriptor fixes the problem ...

But this should NOT be a manual process at all ... hope you can make a fix.

Also ... really really hope this will be implemented: https://github.com/flipboxfactory/saml-sp/issues/47

dsmrt commented 4 years ago

I understand removing the RoleDescriptor node is fixing the issue but i need to know what the original issue is to fix. A RoleDescriptor node present in the metadata shouldn’t be a problem.

I suspect the issue is due to the metadata being larger than the db column supports. I’ve been notified of this issue before but have avoided it do to the migration that is required to alter the column. Can you verify that this is your issue?

Also, #47 would fit nicely with this patch so I’d hope to get these out together.

syska commented 4 years ago

@dsmrt

WOW, quick feedback.

Size: 72,8 KB (74.576 bytes) ... not sure what the Size limit is ...

This site is hosted on https://hyperlane.co if that matters. ( I'm not a CraftCMS ekspert, just the ADFS guy. )

But that site I linked to says it's invalid, hence the reason I removed it and then it worked.

I can send you the link to the ADFS Metadata file ...

dsmrt commented 4 years ago

Ok. It seems like file size is the issue here. I assume you are using MySQL which gives the basic TEXT columns 64KB max. I’m going to need to do some db work for #47 so I’ll add a migration for altering the column to possibly MEDIUMTEXT (16MB max should be enough, I’d think!).

I’ll try and get these 2 issues going within the next couple weeks. It’s good to know you are an ADFS guy. Most have my bugs have come from ADFS and could always have more eyes on what should be improved.

syska commented 4 years ago

@dsmrt

Well ... not actually an ADFS guy, but the only person that was actually able to set it up on CraftCMS ...

Like to see myself as a Digital Swiss Army Knife ... I fix problems no one else is able to.

I guess we ran into this issue between christian and now:

By default, ADFS is configured to generate self-signed token certificates with a duration of one year. This duration can be changed, but keep in mind that the token-signing certificate is the foundation of the sign on process, and therefore, it really shouldn't have a duration longer than 3 years.14. okt. 2017

The reason why the auto update would be awesome.

dsmrt commented 4 years ago

Fixed in 2.1!