shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Demers <brian.dem...@gmail.com>
Subject Re: Defect in WildcardPermission evaluation?
Date Wed, 24 Jan 2018 15:05:30 GMT
Claude

The test I linked to is:
newsletter:*:*:*  => newsletter:read

Which is similar to what you are saying.  I'm all for adding more test to
make this more clear though!

I think part of this issue might be a modeling one (maybe we need to
improve the doc?)

Typically you would assign your users a wildcard (or more loose permission)
and your resource a very specific one.

In your example your printer tech would have `printer:*` (or just
`printer), the 'guru' would have something like `printer:configure,print`

When adding a new printer your application would check for the specific
permission `printer:add`  (this would allow the tech to have access but not
the guru)


You could also add a printer Id to the permissions for the same thing:
The tech would have `printer:*:*`, the guru might have a restricted access
to configure a few printers:
`printer:office-printer1,basement-printer-2:configure`, and also the
permission to print to any printer `printer:*:print`
This would allow the tech to add a printer (resource permission might be:
`printer:*:add`, or `printer:new-id:add`, but the 'guru' could not add any
printer, and could only configure the `office-printer1` or
`basement-printer2` (as well as printing to any printer)

The options are pretty endless depending on how you want to model it, you
could have printer:<location>:<name>:<action>.

Does this help?


On Wed, Jan 24, 2018 at 3:05 AM, Claude Warren <claude@xenei.com> wrote:

> lines 182 - 189 do not cover this issue.
>
> In those lines the pattern can be simplified to "X:*" => X.Y"
>
> I am asking about "X:*" => X
>
> does a longer (more segments) pattern match a shorter wildcard free
> pattern?
>
> I think it should not.
>
> Suppose you have 2 employees.  1 is the printing services tech and the
> other is the printer guru.  Printing services tech can do anything with
> printers, so you give him "printer" permissions.  guru is a wiz at
> configuring printers but you don't want him messing with the print servers
> or installing/removing printers.  So you give guru "printer:*" permissions.
>
> Now you have an operation to add a printer so you protect that by requiring
> "printer" permission.
>
> As expected tech will be able to execute add printer, but so will guru.
>
> The only way, under the current implementation, to limit guru to printers
> only is to list each printer in his permissions list.  This is unexpected
> and unnecessary if the wildcard checking is performed differently.
>
> The change I see is that: until a wild card is encountered: once the
> separator character ":" is found the matching pattern must have a colon.
>
> so that "printer.*" not => printer
>
>
> Claude
>
> On Tue, Jan 23, 2018 at 10:21 PM, Brian Demers <brian.demers@gmail.com>
> wrote:
>
> > Claude,
> > I think I see what you are saying, but it looks like this is what this
> test
> > does:
> > https://github.com/apache/shiro/blob/master/core/src/
> > test/java/org/apache/shiro/authz/permission/WildcardPermissionTest.java#
> > L182-L189
> >
> > Maybe this is something a tweak to the doc can fix?
> >
> > On Tue, Jan 23, 2018 at 5:13 PM, Claude Warren <claude@xenei.com> wrote:
> >
> > > Brian,
> > >
> > > thanks for the response, but the document cited does not cover the
> > question
> > > nor do the test cases.
> > >
> > >
> > > If we take the first two examples in the missing parts section:
> > >
> > > it clearly states that  ( assume the symbol => means implies)
> > >
> > > printer:print => printer:print:*
> > >
> > >
> > > but says noting about
> > >
> > > printer:print:* => printer:print
> > >
> > > I think that this statement is false.  Logically the addition of the
> ":*"
> > > should indicate that permissions are grated to things under
> > "printer:print"
> > > but not to "printer:print" itself.
> > >
> > >
> > > I hope this makes sense.
> > >
> > > Claude
> > >
> > >
> > >
> > >
> > >
> > >
> > > On Tue, Jan 23, 2018 at 2:48 PM, Brian Demers <brian.demers@gmail.com>
> > > wrote:
> > >
> > > > Hey Claude, I thought someone responded to this already (sorry)
> > > >
> > > > To me, this seems in line with the doc:
> > > > https://shiro.apache.org/permissions.html#missing-parts
> > > > At first glance, there are similar test cases in
> > `WildcardPermissionTest
> > > > <https://github.com/apache/shiro/blob/master/core/src/
> > > > test/java/org/apache/shiro/authz/permission/
> > WildcardPermissionTest.java#
> > > > L143>
> > > > `
> > > >
> > > > Does that help clear things up? If not maybe a PR to that test class
> > > would
> > > > help clarify the point?
> > > >
> > > > Thanks!
> > > > -Brian
> > > >
> > > >
> > > > On Tue, Jan 23, 2018 at 5:36 AM, claude.warren@wipro.com <
> > > > claude.warren@wipro.com> wrote:
> > > >
> > > > > Not seeing any discussion of this and seeing no tests in the test
> > cases
> > > > > that perform any tests of this issue.  I will open a defect and
> work
> > on
> > > > > that.
> > > > >
> > > > >
> > > > > Claude
> > > > >
> > > > > ________________________________
> > > > > From: Claude Warren (Product Engineering Service)
> > > > > Sent: Friday, January 12, 2018 12:52:52 PM
> > > > > To: dev@shiro.apache.org
> > > > > Subject: Defect in WildcardPermission evaluation?
> > > > >
> > > > >
> > > > > Currently the WildcardPermission.implies() method contains the
> > > following
> > > > > code snippet and comment.
> > > > >
> > > > >
> > > > > // If this permission has less parts than the other permission,
> > > > everything
> > > > > after the number of parts contained
> > > > > // in this permission is automatically implied, so return true
> > > > >  ....
> > > > >
> > > > >
> > > > > // If this permission has more parts than the other parts, only
> imply
> > > it
> > > > > if all of the other parts are wildcards
> > > > >         for (; i < getParts().size(); i++) {
> > > > >             Set<String> part = getParts().get(i);
> > > > >             if (!part.contains(WILDCARD_TOKEN)) {
> > > > >                 return false;
> > > > >             }
> > > > >         }
> > > > >
> > > > >
> > > > > This means that If you have (User perms in first col, testing
> against
> > > > > across columns)
> > > > > {noformat}
> > > > >
> > > > >
> > > > >         A       A:*     A:B     A:B:*   A:B:C   A:B:C:*
> > > > > A       t       t       t       t       t       t
> > > > > A:*     T
> > > > >         t       t       t       t       t
> > > > > A:B     f       f       t       t       t       t
> > > > > A:B:*   f       f       T       t       t       t
> > > > > A:B:C   f       f       f       f       t       t
> > > > > A:B:C:* f       f       f       f       T       t
> > > > >
> > > > > {noformat}
> > > > >
> > > > > I think the issues are where the upper case  "T"s are.   I believe
> > that
> > > > > those should be "F"
> > > > >
> > > > > The logic being that once a separator (:) is presented it should
no
> > > > longer
> > > > > match anything shorter than that.
> > > > >
> > > > > Thoughts?
> > > > > Claude
> > > > >
> > > > >
> > > > > The information contained in this electronic message and any
> > > attachments
> > > > > to this message are intended for the exclusive use of the
> > addressee(s)
> > > > and
> > > > > may contain proprietary, confidential or privileged information.
If
> > you
> > > > are
> > > > > not the intended recipient, you should not disseminate, distribute
> or
> > > > copy
> > > > > this e-mail. Please notify the sender immediately and destroy all
> > > copies
> > > > of
> > > > > this message and any attachments. WARNING: Computer viruses can be
> > > > > transmitted via email. The recipient should check this email and
> any
> > > > > attachments for the presence of viruses. The company accepts no
> > > liability
> > > > > for any damage caused by any virus transmitted by this email.
> > > > > www.wipro.com
> > > > >
> > > > > ____________________________________________________________
> > __________
> > > > > This email has been scanned by the Symantec Email Security.cloud
> > > service.
> > > > > For more information please visit http://www.symanteccloud.com
> > > > > ____________________________________________________________
> > __________
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > I like: Like Like - The likeliest place on the web
> > > <http://like-like.xenei.com>
> > > LinkedIn: http://www.linkedin.com/in/claudewarren
> > >
> >
>
>
>
> --
> I like: Like Like - The likeliest place on the web
> <http://like-like.xenei.com>
> LinkedIn: http://www.linkedin.com/in/claudewarren
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message