-
Notifications
You must be signed in to change notification settings - Fork 0
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
Allow blank metadata #8
Conversation
@@ -9,7 +9,7 @@ class SamlMetaData(models.Model): | |||
blank=True, | |||
) | |||
created_at = models.DateTimeField(auto_now_add=True) | |||
metadata_contents = models.TextField() | |||
metadata_contents = models.TextField(blank=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this a validation that only happens on django admin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the same but with TextField/CharField it makes it also so that '' is allowed. With other field types it's just django admin validation
@@ -172,6 +172,10 @@ def acs(r, metadata_id): | |||
logger.warning("Denied because missing metadata") | |||
return HttpResponseRedirect(get_reverse([denied, 'denied', 'django_saml2_auth:denied'])) | |||
|
|||
if metadata.metadata_contents == '': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we allowing or denying?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol it is misleading, what I meant is we want to allow blank metadata at the database level and then don't throw 500 errors
From this: https://app.asana.com/0/1205521539367651/1205389957689159
Basically Alex needs to create the SAMLMetaData config in order to get the ACS url to send to the client, but Alex doesn't have the metadata yet. If someone accesses this URL it will throw a 500 (https://console.cloud.google.com/errors/detail/CLXpuafIpquoDg;time=P30D?project=passthrough-prod&utm_source=error-reporting-notification&utm_medium=email&utm_content=resolved-error&pli=1&rapt=AEjHL4MQ-boCDg-X4YpXJeZkR-siEVwhKxTIIS_5mvKlciITJmwjQiVA2eCzzlyLyvO8JBLc9CyHmYrJCuD9zU-3EuICVihkCQ)
Allowing a blank metadata at the DB level and then denying it when someone accesses the URL will prevent this 500 and fail gracefully. No one should use the link anyway until there's a metadata