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

API Log Integrity #3

Merged
merged 2 commits into from
Feb 27, 2020
Merged

Conversation

cyclops26
Copy link

Added support for persistent usernames (if user model object is deleted). Added support for the log to be read-only in Django admin. Defaults are configured for full legacy support.

These are two "security" related patches that I find myself constantly backloading into projects with this package. Figured I would clean it up into something that others might find useful.

@lingster lingster merged commit e8812a9 into lingster:master Feb 27, 2020
@lingster
Copy link
Owner

Ok merged - just wondering if it would be better to make read-only the default for viewing access logs in the admin screen?

@cyclops26
Copy link
Author

@lingster ultimately yes, from a security standpoint it would be best to default to "read-only". I did not default to that initially because of backwards compatibility. By defaulting to still writeable, you can essentially not use any of the new config and your package will continue to work as it has previously.

I find that most package maintainers prefer to introduce a new feature (unless it is a major security risk) as a non-breaking change (with a notice or warning) first and then in the next release moving it to a more secure code/config option.

That gives developers time to become aware and make changes to their systems/processes as necessary.

@cyclops26 cyclops26 deleted the admin-log-integrity branch February 27, 2020 22:05
@lingster
Copy link
Owner

lingster commented Mar 3, 2020

I guess as this is a fork of the original, maybe we can just enable read-only in the next release and put a note in the release notes...

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

Successfully merging this pull request may close these issues.

3 participants