Project

General

Profile

Bug #15108

Cloned role has no builtin value

Added by Paul Smyth about 4 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Authorization
Target version:
Difficulty:
Triaged:
Bugzilla link:
Fixed in Releases:
Found in Releases:

Description

Creating a new role from the Administer table by cloning an existing role results in no value being entered into the 'builtin' field. This makes the role non selectable in the user and groups menus and not deletable. Work around is to update the roles table directly eg: "update roles set builtin = 0 where id = xx;".

Note: This is 1.11.2 which is not selectable below so please accept my apologies if this has been fixed in 1.11.3


Related issues

Related to Foreman - Refactor #15621: Change role builtin attribute to booleanNew2016-07-08
Has duplicate Foreman - Bug #15520: Delete cloned role via APIDuplicate2016-06-24

Associated revisions

Revision 919cde52 (diff)
Added by Marek Hulán almost 4 years ago

Fixes #15108 - allow editing cloned roles

History

#1 Updated by Paul Smyth about 4 years ago

Should say "Administer menu"

#2 Updated by Dominic Cleal about 4 years ago

  • Category set to Authorization
  • Legacy Backlogs Release (now unused) deleted (159)

I'm not able to reproduce this, albeit on nightly/1.12 (which shouldn't really be any different in this regard). The entire role including the old role's builtin field should be copied.

Edit: actually, builtin is even explicitly set to false during creation.

#3 Updated by Paul Smyth about 4 years ago

This is the output of "select * from roles" after cloning the role "viewer" into "New test viewer":
id | name | builtin | permissions
----+--------------------------+---------+-------------
1 | Manager | 0 |
2 | Edit partition tables | 0 |
3 | View hosts | 0 |
4 | Edit hosts | 0 |
5 | Viewer | 0 |
6 | Site manager | 0 |
7 | Default user | 1 |
8 | Anonymous | 2 |
9 | Provisioning setup | 0 |
10 | Discovery Reader | 0 |
11 | Discovery Manager | 0 |
12 | Tasks Manager | 0 |
13 | Tasks Reader | 0 |
14 | Remote Execution User | 0 |
15 | Remote Execution Manager | 0 |
16 | Developer | 0 |
20 | New test viewer | |

As you can see there is no value in the 'builtin' column. This is simply cloning without altering any filters

#4 Updated by Paul Smyth about 4 years ago

Looking at the table schema, the 'builtin' field is an integer, not a boolean. Is it possible that false is not 'FALSE' in this instance therefore not applying the value zero to the field?

Column    |          Type          |                     Modifiers                      | Storage  | Stats target | Description 
-------------+------------------------+----------------------------------------------------+----------+--------------+-------------
id | integer | not null default nextval('roles_id_seq'::regclass) | plain | |
name | character varying(255) | | extended | |
builtin | integer | | plain | |
permissions | text | | extended | |
Indexes:
"roles_pkey" PRIMARY KEY, btree (id)
Referenced by:
TABLE "filters" CONSTRAINT "filters_roles_id_fk" FOREIGN KEY (role_id) REFERENCES roles(id)
TABLE "user_roles" CONSTRAINT "user_roles_role_id_fk" FOREIGN KEY (role_id) REFERENCES roles(id)
Has OIDs: no

#5 Updated by Simon Mügge about 4 years ago

Can confirm this behaviour too, using 1.11.2

#6 Updated by Dominic Cleal about 4 years ago

  • Status changed from New to Resolved

Thanks, I see it as well now I'm testing on 1.11.2 and it does seem to have disappeared in 1.12.0-RC1, perhaps because we updated Rails or another component. Sorry I missed that earlier.

Since I think it's resolved in 1.12 without a Foreman code change, I'll close this for now, but if somebody wants/knows how to write a 1.11-specific fix then we can re-open it for a patch release.

(A schema change defaulting builtin to 0 would be a possible fix and tidyup, but wouldn't be suitable for a patch release.)

#7 Updated by Dominic Cleal about 4 years ago

  • Has duplicate Bug #15520: Delete cloned role via API added

#8 Updated by Dominic Cleal almost 4 years ago

  • Related to Refactor #15621: Change role builtin attribute to boolean added

#9 Updated by Marek Hulán almost 4 years ago

  • Status changed from Resolved to Assigned
  • Assignee set to Marek Hulán
  • Legacy Backlogs Release (now unused) set to 175

This is still reproducible in 1.12. The fix was introduced by this migration I'll open a PR fixing it in 1.12 without schema change. In 1.13+ the fix sets builtin value to 0 by default.

#10 Updated by The Foreman Bot almost 4 years ago

  • Status changed from Assigned to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/3706 added

#11 Updated by Dominic Cleal almost 4 years ago

Marek Hulán wrote:

This is still reproducible in 1.12. The fix was introduced by this migration

I think both my comment above (#6) and the observations on
https://github.com/theforeman/foreman/pull/3630#issuecomment-231667333 were made prior to this change in develop. It probably did help (as will #15621's refactoring), but the issue did seem to have disappeared in 1.12.0. Probably a conversation to continue with the PR reviewer.

#12 Updated by Marek Hulán almost 4 years ago

I reproduced that issue with 1.12-stable version. While cloning role on 1.12 works fine since false is set, if there are roles from previous version with nil builtin value, it's still broken.

#13 Updated by Marek Hulán almost 4 years ago

  • Target version set to 1.6.3

#14 Updated by Dominic Cleal almost 4 years ago

  • Status changed from Ready For Testing to Closed

Fixed in 919cde52ef017819767dabd0424a6c6bb6c9e788.

#15 Updated by Shimon Shtein about 3 years ago

  • Bugzilla link set to 1457658

Also available in: Atom PDF