15five / django-scim2

A SCIM 2.0 Service Provider Implementation (for Django)
http://django-scim2.readthedocs.io/
Other
76 stars 27 forks source link

SCIMView.dispatch() should allow opting out of putting all exceptions' details in the response #59

Closed mateuszmandera closed 2 years ago

mateuszmandera commented 2 years ago

The SCIMView.dispatch method:

    @method_decorator(csrf_exempt)
    @method_decorator(login_required)
    def dispatch(self, request, *args, **kwargs):
        if not self.implemented:
            return self.status_501(request, *args, **kwargs)

        try:
            return super(SCIMView, self).dispatch(request, *args, **kwargs)
        except Exception as e:
            if not isinstance(e, exceptions.SCIMException):
                logger.exception('Unable to complete SCIM call.')
                e = exceptions.SCIMException(str(e))

            content = json.dumps(e.to_dict())
            return HttpResponse(content=content,
                                content_type=constants.SCIM_CONTENT_TYPE,
                                status=e.status)

converts any exception into a string and puts it in the HttpResponse. I think ideally there'd be a setting allowing library users to opt out of this behavior (or possibly even better - this behavior should be disabled by default and require explicitly enabling it) and use a generic message such as Exception occurred while processing the SCIM request. The reason being that while this is very useful for debugging, it can be dangerous in production - sensitive information can be revealed through str(e) and the SCIM client may not always be a party so trusted that revealing information would be harmless.

As a simple example if you deploy some bug to production where some_dict[secret_string] gets called causing KeyError, the secret_string will be revealed to the SCIM client.

logston commented 2 years ago

Thanks for the issue report! Currently very busy at my day job so it might be a bit before I can get to this. If you'd like to do so instead, please feel free; just post here that you are working on the issue.

powellc commented 2 years ago

@logston I've been fixing issues like this in our codebase with hacks. I'm happy to open a PR to resolve this, as it's probably a better place to fix sensitive data leaks.

logston commented 2 years ago

Sure! If you'd like to take it, by all means. Let me know your decision @powellc.

mateuszmandera commented 2 years ago

Thanks a lot for the fix!