Page MenuHomePhabricator

"changed Security from none to none." in initial task creation
Closed, ResolvedPublic

Tokens
"Heartbreak" token, awarded by PiRSquared17."Like" token, awarded by He7d3r."Like" token, awarded by Ricordisamoa."Like" token, awarded by jeremyb.
Assigned To
Authored By
Qgil, Sep 25 2014

Description

When a user creates a task, this entry appears in the timeline:

Qgil changed Security from none to none.

Can this entry be avoided in the future? No need to change anything in tasks that already have it.

Details

Commits
Unknown Object (Diffusion Commit)
Unknown Object (Diffusion Commit)
Related Gerrit Patches:

Event Timeline

Qgil created this task.Sep 25 2014, 10:04 AM
Qgil raised the priority of this task from to Needs Triage.
Qgil updated the task description. (Show Details)
Qgil added a project: Phabricator.
Qgil changed Security from none to None.
Qgil added a subscriber: Qgil.

I'm not exactly sure why this is happening. I looked through the security plugin code and I don't see anything that would be causing this.

Qgil added a comment.Sep 29 2014, 6:47 AM

No need to "+1" when you can "Award Token". :)

mmodell triaged this task as Low priority.Oct 4 2014, 3:51 PM
Qgil added subscribers: Unknown Object (MLST), Nemo_bis.
Qgil added a subscriber: Legoktm.Oct 11 2014, 11:02 PM

apparently this is not just a problem at initial filing time. T109#10883:

jeremyb-phone set Security to none.

apparently this is not just a problem at initial filing time.

Yes, it also affects tickets that were imported or created before setting up the specific Security field values.

Ricordisamoa added a subscriber: Ricordisamoa.
Qgil raised the priority of this task from Low to Normal.Nov 7 2014, 10:31 AM

Do we have any idea whether the Bugzilla and RT migrated tasks will inherit this problem as well?

Do we have any idea whether the Bugzilla and RT migrated tasks will inherit this problem as well?

Can this be tested on the bugzilla preview?

TTO added a subscriber: TTO.Nov 23 2014, 11:57 PM

Imported BZ tasks didn't get "changed Security from none to none", thankfully. But the first time you use "Edit Task", Phabricator thinks that you "set Security to none", which is equally annoying. See T58091 for an example.

mmodell claimed this task.Nov 24 2014, 3:21 AM

This is annoying but I've tried everything I can think of to make it go away and no matter what I've tried phab still does some form of this logging in the task transactions. It's dumb and I really don't know what the solution is but I'll keep this on my radar.

mmodell lowered the priority of this task from Normal to Low.Nov 24 2014, 3:22 AM
Qgil added a comment.Nov 24 2014, 8:14 AM

Is it worth to ask for a second pair of eyes upstream? It is just a detail but it is indeed annoying.

In T479#777578, @Qgil wrote:

Is it worth to ask for a second pair of eyes upstream? It is just a detail but it is indeed annoying.

Yes.

He7d3r added a subscriber: He7d3r.
Qgil raised the priority of this task from Low to Normal.Dec 3 2014, 9:51 AM

This problem triggered in a any new or migrated task plus T76008 is more than a cosmetic problem. Increasing priority. @mmodell, if you think your code is fine and this is something that Herald or something else is doing wrong, I can file a task upstream.

As an intermediate solution, could we have a bot go through and null edit every task to mark all the tasks that are currently public as Security = none.

This currently feels like death by a thousand cuts. If Phabricator/Maniphest insists that every task have a security policy, can we just do a single pass with a bot and get this over with?

Qgil added a comment.Dec 11 2014, 8:29 AM

Please let us fix T493 (should be done one of these days) and see what happens with this problem.

mmodell added a comment.EditedDec 11 2014, 10:25 PM

I finally got to the bottom of the "none" to "none" issue:

The field configuration has "default": "none"
And the options also has "default": "none"

however, the value of "default" is supposed to point to the key of the entry in options, so
we need to change it to "default": "default" as in the following field config:

 "security_topic" : {
   "default" : "default",
   "description" : "Used for security oriented custom extensions",
   "instructions" : "Security settings will override permissions and projects as needed.",
   "name" : "Security",
   "options" : {
     "default" : "none",
     "sensitive" : "Private Issue",
     "security-bug" : "Security Bug"
   },
   "search" : true,
   "type" : "select"
}

Change 179407 had a related patch set uploaded (by 20after4):
change "default: none" to "default: default" for phabricator's security_topic field to fix the "changed none to none" bug. This fixes T479

https://gerrit.wikimedia.org/r/179407

Patch-For-Review

Amire80 removed a subscriber: Amire80.Dec 12 2014, 7:34 PM

This is extra confusing when a task has a non-public view policy. Security is none but also it's not visible to a typcial user. (maybe off-topic for this task. feel free to point me somewhere else)

Qgil added a comment.Dec 19 2014, 7:25 AM

@jeremyb, can you post here the number of the task you are referring to?

non-public tasks should have the Security field set to something other than "none", as it is the case reported in T76008 (a blocked task that I hope is solved by the same patch under review for this one).

look at almost any ops-core task

@Qgil & @jeremyb-phone: The patch linked above will resolve the issue of confusing entries in the task transaction log. It will not prevent you from setting a custom policy yet leaving the security field set to none. You shouldn't need to use the custom policy interface, just set the security field and let phabricator take care of generating the custom policy for you.

Change 179407 merged by Rush:
phabricator: Change security_topic from "default: none" to "default: default"

https://gerrit.wikimedia.org/r/179407

chasemp mentioned this in Unknown Object (Diffusion Commit).Dec 22 2014, 6:42 PM

The "USERNAME set Security to none" action is still happening for tasks imported from bugzilla, and is confusing some users. See T20871#983927

As an intermediate solution, could we have a bot go through and null edit every task to mark all the tasks that are currently public as Security = none.
This currently feels like death by a thousand cuts. If Phabricator/Maniphest insists that every task have a security policy, can we just do a single pass with a bot and get this over with?

There's a plan to do this or has it been declined?

Nemo_bis reopened this task as Open.Feb 12 2015, 10:03 PM

This isn't fixed on new tasks either. https://phabricator.wikimedia.org/T89398#1035387

scfc closed this task as Resolved.Feb 12 2015, 10:31 PM
scfc added a subscriber: PiRSquared17.

I think @PiRSquared17 used "Edit Task" to subscribe you and thus triggered T87135. This task is only about the initial creation of a task which worked fine for T89398.

Nemo_bis reopened this task as Open.Feb 13 2015, 9:42 AM

Yes, and the string is slightly different, but the difference is negligible.

Nemo_bis renamed this task from "changed Security from none to none." to "Security from none to none" and "Security to None" clutter.Feb 13 2015, 9:44 AM

Nemo, I don't understand why you insist on keeping two tasks about one issue open.

scfc added a comment.Feb 13 2015, 2:05 PM

No. There were two errors triggered (likely) by different code paths:

  1. "Qgil changed Security from none to None." in the initial task creation (like in the timeline of this task). This has been fixed, and thus this task should be closed.
  2. "scfc set Security to None." in subsequent uses of "Edit Task" (like in the timelines of T87135 or T89398). This has not been fixed, and is covered by T87135.

Mixing this up in one task is not aiding fixing them as then energy is lost in deciphering what already has been resolved and what still needs to be fixed.

matmarex closed this task as Resolved.Feb 13 2015, 2:13 PM

Let's continue the discussion at T87135 then.

matmarex renamed this task from "Security from none to none" and "Security to None" clutter to "changed Security from none to none.".Feb 13 2015, 2:14 PM
Aklapper renamed this task from "changed Security from none to none." to "changed Security from none to none." in initial task creation.Feb 13 2015, 6:52 PM
Diffusion added a commit: Unknown Object (Diffusion Commit).Mar 4 2015, 8:15 AM
Diffusion added a commit: Unknown Object (Diffusion Commit).
Qgil added a comment.Mar 4 2015, 8:53 AM

Accidental clash. Known issue. Sorry for the noise.