shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Claude Warren <cla...@xenei.com>
Subject Re: Defect in WildcardPermission evaluation?
Date Wed, 24 Jan 2018 08:05:49 GMT
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