Opened 5 years ago

Closed 5 years ago

#17460 closed Cleanup/optimization (fixed)

add KEY to HIDDEN_SETTINGS regexp

Reported by: daenney Owned by: chomik
Component: Core (Other) Version: 1.4-alpha-1
Severity: Normal Keywords: hidden settings, hidden_settings, hide settings
Cc: lpiatek Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

The HIDDEN_SETTINGS regexp is very useful but currently doesn't do much to obfuscate things such as API_KEY settings which are becoming more and more common to hook into additional services.

This would also be solved if/when #16435 gets resolved as it would allow people to add KEY to that regexp themselves.

However, for now and because the usage of such API key's is mainstream, I think it would be a good idea to add KEY to the default HIDDEN_SETTINGS regexp.

Attachments (5)

HIDDEN_SETTINGS.patch (490 bytes) - added by daenney 5 years ago.
django-17460-hidden_settings.patch (509 bytes) - added by Bas Peschier 5 years ago.
hidden_settingsv2.patch (1.7 KB) - added by daenney 5 years ago.
hidden_settings3.patch (1.6 KB) - added by Tomasz Rybak 5 years ago.
Fixed doc to be the same as regexp
hidden_settings4.patch (50.7 KB) - added by chomik 5 years ago.
patch with improved documentation and release notes for 1.4-beta-1

Download all attachments as: .zip

Change History (18)

comment:1 Changed 5 years ago by daenney

Easy pickings: set
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 5 years ago by daenney

TOKEN and PASS would be two other fairly common used names for such variables.

Changed 5 years ago by daenney

Attachment: HIDDEN_SETTINGS.patch added

comment:3 Changed 5 years ago by daenney

Has patch: set

Added patch.

comment:4 Changed 5 years ago by Gabriel Hurley

Component: UncategorizedCore (Other)
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

I'm a little loathe to see more work happen on HIDDEN_SETTINGS when it's such an ugly solution to begin with. Especially now that we have custom error reporting. It'd be so much better to see this be part of a common solution to #16435 as noted.

However, those are common sensitive values, so I'll accept the ticket for now.

Changed 5 years ago by Bas Peschier

comment:5 Changed 5 years ago by Bas Peschier

Triage Stage: AcceptedReady for checkin

Cleaned up the diff so it patches from the root; please remember that the next time you write a patch, it just makes it easier to review the ticket.

I agree with the "should be fixed with a better solution all together", but it does not make the HIDDEN_SETTINGS worse by adding a few good values.

comment:6 Changed 5 years ago by Aymeric Augustin

Triage Stage: Ready for checkinAccepted

Note that PASS and PASSWORD are redundant in the regexp.

I'm not sure if this needs tests (probably not) or docs (two lines in the "backward-incompatible changes" section of the release notes?).

Last edited 5 years ago by Aymeric Augustin (previous) (diff)

comment:7 Changed 5 years ago by Bas Peschier

Patch needs improvement: set

Good catch, the docs actually *do* specify which settings get hidden, so updated docs are needed. A mention in the release notes is a good idea as well.

Changed 5 years ago by daenney

Attachment: hidden_settingsv2.patch added

comment:8 Changed 5 years ago by daenney

Attached a newer version of the patch that also updates the docs to reflect the changes.

comment:9 Changed 5 years ago by daenney

There's another little issue with the docs, the docs say anything that contains PROFANITIES will match but the HIDDEN_SETTINGS regexp specifies PROFANITIES_LIST as it's pattern so as far as I know PROFANITIES_CAKE won't match in that case.

Changed 5 years ago by Tomasz Rybak

Attachment: hidden_settings3.patch added

Fixed doc to be the same as regexp

comment:10 Changed 5 years ago by Tomasz Rybak

Patch needs improvement: unset

Now documentation matches regexp HIDDEN_SETTINGS from patch (hidden_settings3.patch).

comment:11 Changed 5 years ago by chomik

Owner: changed from nobody to chomik

Changed 5 years ago by chomik

Attachment: hidden_settings4.patch added

patch with improved documentation and release notes for 1.4-beta-1

comment:12 Changed 5 years ago by lpiatek

Cc: lpiatek added
Keywords: hidden settings hidden_settings hide settings added
Triage Stage: AcceptedReady for checkin

Patch also adds release notes for 1.4-beta-1, which are basicly release notes from 1.4-aplha-1 with aplpha to beta changes where necessary.

I think it is ready for check in.

comment:13 Changed 5 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

In [17481]:

(The changeset message doesn't reference this ticket)

Note: See TracTickets for help on using tickets.
Back to Top