Details

    • Type: Task Task
    • Status: Closed Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: <= 3.x
    • Labels:
      None
    • Sub-Committee:
      Tech / QA / PM
    • Impacted Modules:
      System

      Description

      no functional change - the one that needed to take the bo was canCreateOrMaintain - see g and h here: https://test.kuali.org/jira/browse/KFSMI-2110
      i was referring to the submit part. which is what canCreateOrMaintain was for. and, that's the only method signature i modified. i actually made it take the doc instead of the bo, so that we could use the isAuthorized method on document authorizer base - initially it wasn't even taking the bo - just the class and pks.

      you're right that the canCreate and canMaintain methods were for the lookups, and i didn't muck with those signatures. those looked right. the only thing i did with those methods was make some notes on the fact that we would need to populate the permission details ourselves and call kim directly like the canInitiate, since the isAuthorized* methods on document authorizer base will blow up without a document.

      hope that helps. i'll check in when i get back.

      thanks!
      a

      ________________________________________
      From: Warren Liang [wliang@uci.edu]
      Sent: Thursday, December 25, 2008 1:58 AM
      To: Byrne, Ailish M
      Subject: Re: ok...

      On KFSMI-2089, we wrote that:

      d. check the canCreate method from the lookupable to determine
      whether to display the create new button
      e. check the canCreate method lookupable helper service when
      determining whether to display the copy link
      f. check the canMaintain method from the lookupable helper service
      when determining whether to display the edit link

      I assumed that the above was to help render the lookup results to decide
      whether to render the new/copy/edit link. Then, for the submit rule we
      would also check create/maintain as well.

      Is there a functional change? i.e. we're going to render the
      create/maintain buttons regardless of authorization, and then if they
      try to edit a record they're not allowed to, then we display an error
      when they submit.

      Thanks,

      Warren

      Byrne, Ailish M wrote:
      > > My recollection is that the create or maintain was to be used from the submit rule per our original discussion / the jira. It needs to be cause in our use cases cams needs to pull the chart and org from a new asset - can't be obtained via pk. I found when I modified the signature that the rule is already calling the method - lmk what u think. Not trying ti be all up in ur biz - just trying to make sure we accomodate our use cases and the devs and am under a lot of pressure to help them get their work done given the rapidly approachng qa date. There is no way we would be half of where we are without u constantly going double time and beyond
      > > Thanks
      > > A
      > >
      > > ----------------------
      > > Sent from my blackberry
      > >
      > > ----- Original Message -----
      > > From: Warren Liang <wliang@uci.edu>
      > > To: Byrne, Ailish M
      > > Sent: Thu Dec 25 01:03:32 2008
      > > Subject: Re: ok...
      > >
      > > Hey Ailish,
      > >
      > > Sorry I didn't get back to you earlier on this. I took a look through
      > > your patch, and I noted that in several circumstances, you changed
      > > methods such as this:
      > >
      > > public boolean canCreateOrMaintain(Class boClass, Map primaryKeys,
      > > Person user)
      > >
      > > to
      > >
      > > public final boolean canCreateOrMaintain(MaintenanceDocument
      > > maintenanceDocument, Person user).
      > >
      > > I intentionally made the signature to include the class and the keys
      > > because the method is used by the lookup framework, and the lookup
      > > framework doesn't have a document instance. Instead of passing a class
      > > and a map, we could pass in a BO, but I don't see how we could
      > > reasonably pass in a Maintenance Document.
      > >
      > > I'll try to commit my changes into Rice/KFS tonight.
      > >
      > > Thanks,
      > >
      > > Warren
      > >
      > > Byrne, Ailish M wrote:
      > >
      >> >> Anything related to the APIs that the devs in other modules working to implement this have to work with is the highest priority. I'm ok saying certain things haven't been implemented, but I really want to be clear about what APIs, they should be using.
      >> >>
      >> >> I see the API for the "ok..." message below, i.e. the maintenance and inquiry presentation controller logic for fields and sections is not done. I went ahead and put in the part for the MaintenanceDocumentPresentationController and authorizer. can you keep me posted on the rest.
      >> >>
      >> >> I think canInitiate and canReceiveAdHoc should be final on DocumentAuthorizerBase and the protected can* methods should go away. We should not assume / facilitate people overriding the KIM template checks.
      >> >>
      >> >> We need canDisapprove on the presentation controller. There are a couple docs that just flat out say no to this.
      >> >>
      >> >> DocumentAuthorizerBase appears to be ignoring canEdit from the presentation controller.
      >> >>
      >> >> DocumentAuthorizerBase was never checking canComplete. So, I removed it. I think we should just ditch this one given time and the fact that we found that it's only set in conjunction with approve.
      >> >>
      >> >> I deprecated hasInitiateAuthorization.
      >> >>
      >> >> TransactionalDocumentAuthorizerBase was not checking if an edit mode permission existed which conflicted what we told people to do in terms of presentation controller implementation, so i fixed that
      >> >>
      >> >> Jonathan and I talked the other day about how we think canInitiate should just be returning a boolean, and the framework should throw the exception. Also, the error message for that exception hasn't been updated, and the output doesn't make any sense. Now that users will be able to look at who has the initiate permission in KIM, I think the message should refer to a permission, not the workgroup / role.
      >> >>
      >> >> I changed the method signature of MaintenanceDocumentAuthorizer.canCreateOrMaintain, because it didn't match the instructions in the jira and didn't make sense to me. note that i also included a helper method they can implement to get the qualifications from the bo.
      >> >>
      >> >> Here's the page that I'm working on as I was going through this - final documentation for the other modules: https://test.kuali.org/confluence/display/KULKFS/KIM+Impacts. I'll check in when I get back on the 28th. to see whether the apis match or i need to correct the page
      >> >>
      >> >> I've attached a patch with the modifications i thought i could make without breaking client apps, because i didn't want to run into you.
      >> >>
      >> >> Thanks,
      >> >> a
      >> >>
      >> >> ________________________________________
      >> >> From: Byrne, Ailish M
      >> >> Sent: Saturday, December 20, 2008 11:40 PM
      >> >> To: Warren Liang
      >> >> Subject: RE: ok...
      >> >>
      >> >> responses below with 3 stars
      >> >>
      >> >> ________________________________________
      >> >> From: Warren Liang [wliang@uci.edu]
      >> >> Sent: Saturday, December 20, 2008 11:32 PM
      >> >> To: Byrne, Ailish M
      >> >> Subject: Re: ok...
      >> >>
      >> >> Hey Ailish,
      >> >>
      >> >> Responses below, with 2 *'s.
      >> >>
      >> >> Thanks,
      >> >>
      >> >> Warren
      >> >>
      >> >>
      >> >> Byrne, Ailish M wrote:
      >> >>
      >> >>
      >>> >>> see below... hope this helps
      >>> >>> thanks,
      >>> >>> a
      >>> >>>
      >>> >>> ________________________________________
      >>> >>> From: Warren Liang [wliang@uci.edu]
      >>> >>> Sent: Saturday, December 20, 2008 10:48 PM
      >>> >>> To: Byrne, Ailish M
      >>> >>> Subject: Re: ok...
      >>> >>>
      >>> >>> Hey Ailish,
      >>> >>>
      >>> >>> Responses below.
      >>> >>>
      >>> >>> Some questions I have:
      >>> >>>
      >>> >>> 1) do we need the ability to do row-level restrictions on a collection?
      >>> >>> For example, we have a list of account numbers, and only the fiscal
      >>> >>> officer of the account may edit that collection element. If so, this
      >>> >>> might change our plan, as we need to consider how to pass row-specific
      >>> >>> permission details/qualifiers.
      >>> >>>
      >>> >>> * not in maintenance documents now. we can enhance later to pass the pk or give the authorizer a hook if need be.
      >>> >>>
      >>> >>> 2) did we agree that we are not going to restrict collections? The
      >>> >>> implementer may either restrict all fields in a collection, or may
      >>> >>> restrict the section that renders the collection.
      >>> >>>
      >>> >>> * i think doing fields or sections will meet our needs for now. we can enhance later if need be
      >>> >>>
      >>> >>> Thanks,
      >>> >>>
      >>> >>> Warren
      >>> >>>
      >>> >>> Byrne, Ailish M wrote:
      >>> >>>
      >>> >>>
      >>> >>>
      >>>> >>>> So, here's what I think we said was gonna happen at our meeting the other day...
      >>>> >>>>
      >>>> >>>> - I will add section id to the Modify Maintenance Document Field(s) and View Inquiry Or Maintenance Document Field(s) types / templates
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>> >>> Yes
      >>> >>>
      >>> >>> * k
      >>> >>>
      >>> >>>
      >>> >>>
      >>> >>>
      >>>> >>>> - I will change the 3 perms I have that ref colection properties and where we want the corresponding section hidden / read only if the collection is to specify section ids instead
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>> >>> I can only find 2 perm templates. Which is the 3rd that you're
      >>> >>> referring to?
      >>> >>>
      >>> >>> 1) View Inquiry or Maintenance Document Field(s)
      >>> >>> 2) Modify Maintenance Document Field(s)
      >>> >>>
      >>> >>> I assume that the attribute name will be something like "sectionId".
      >>> >>>
      >>> >>> * yes, the attribute name will be sectionId - you can go ahead and add it to KimAttributes now. i was referring to the 3 collection "permissions" we have - you're correct - there are only 2 "permission templates"
      >>> >>>
      >>> >>>
      >>> >>>
      >>> >>>
      >>>> >>>> - You will modify the MaintenanceDocumentPresentationController to add
      >>>> >>>> * public Set<String> getHiddenPropertyNames
      >>>> >>>> * public Set<String> getHiddenSectionIds
      >>>> >>>> * public Set<String> getReadOnlyPropertyNames
      >>>> >>>> * public Set<String> getReadOnlySectionIds
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>> >>> I'm not sure that we would want to implement a read-only section. Could
      >>> >>> we obtain equivalent functionality by specifying AttributeSecurity on
      >>> >>> each field in a section?
      >>> >>>
      >>> >>> * ok, yes we can skip the read only section for now. how do we make a collection property read only? just checking on the systax. let's say my collection is called things and the field is id - do i put things.id in the property name on the permission?
      >>> >>>
      >>> >>> As for fields/properties, I was planning to return a map containing the
      >>> >>> property name as the key and the DD defined AttributeSecurity as the
      >>> >>> value. Therefore, the *PropertyNames methods can be replaced by:
      >>> >>> public Map<String, DataDictionaryDefinedRestrictions>
      >>> >>> getFieldRestrictions
      >>> >>> Where DataDictionaryDefinedRestrictions is a class that contains the
      >>> >>> following:
      >>> >>> - AttributeSecurity from BO DD
      >>> >>> - AttributeSecurity from MD DD
      >>> >>> - unconditionalyReadonly from MD Field Def
      >>> >>>
      >>> >>> * not sure if i'm quite following this. i think the method on the presentation controller should not be responsible for attribute security stuff. just non-user based hidden / read only stuff
      >>> >>>
      >>> >>>
      >>> >>>
      >>> >>>
      >> >> ** What about the mask and partial masking restrictions?
      >> >> *** we don't need to allow the pres controller or authorizer to customize those, so i don't see them as applicable in this work
      >> >>
      >> >> ** You are right, DataDictionaryDefinedRestrictions would not need to be created and used.
      >> >> *** k
      >> >>
      >> >> ** I want to make sure we're both using the same assumptions. Are you assuming that the PresController will look for all unconditional restrictions. The authorizer needs to parse the DD for any user-based authorizations, and will check the permissions if there is not an unconditional restriction defined already.
      >> >> *** no - i am assuming tha pres controller will only return additional restrictions based on document data or state based logic - not get the stuff that's already specified in the dd and return that. same goes for the authorizer - basically the only reason we have the methods on the authorizer is cause we're not adding a way for them to flag the section as secure in the dd. so, given that and the fact that we don't want to do the read only section thing, probably the only method on the authorizer should be getSecurePotentiallyHiddenSectionIds, since everything the property ones should be able to be done in the dd
      >> >>
      >> >>
      >> >>
      >>> >>> What about the mask and partial masking restrictions?
      >>> >>>
      >>> >>>
      >>> >>>
      >>>> >>>> - You will modfy MaintenanceDocumentAuthorizer to add
      >>>> >>>> * public Set<String> getSecurePotentiallyHiddenPropertyNames
      >>>> >>>> * public Set<String> getSecurePotentiallyHiddenSectionIds
      >>>> >>>> * public Set<String> getSecurePotentiallyReadOnlyPropertyNames
      >>>> >>>> * public Set<String> getSecurePotentiallyReadOnlySectionIds
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>> >>> What does "SecurePotentially" mean in the doc authorizeer?
      >>> >>>
      >>> >>> * if i wrote it in english it would be secure, potentially read-only. i.e. secure and potentially are both adjectives. i just meant to say these are the ones that should trigger permission checks.
      >>> >>>
      >>> >>>
      >>> >>>
      >>> >>>
      >> >> ** Isn't the document authorizer responsible for performing the permission checks? If that is the case, what does it mean for the document authorizer to return something that should trigger permission checks, and where would these checks occur?
      >> >> *** again, if we allowed them to flag a section as secure, poptentially hidden in the dd, the authorizer wouldn't need to have any method. and, yes the authorizer is supposed to check the permissions, but i'm focused on the methods they would be writing. the method to check the permissions should be written for them.
      >> >>
      >> >>
      >> >>
      >>> >>> Again, I'm not so sure about the read-only sections.
      >>> >>>
      >>> >>> * no prob - we can skip the read only section
      >>> >>>
      >>> >>> For the *PropertyNames methods, we can return
      >>> >>>
      >>> >>>
      >>> >>>
      >>>> >>>> - You will add code to check the Modify Maintenance Document Field(s) and View Inquiry Or Maintenance Document Field(s) templates based on the set returned by the authorizer calls
      >>>> >>>> - You will add InquiryPresentationController and InquiryAuthorizer classes
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>> >>> Yes.
      >>> >>>
      >>> >>> * k
      >>> >>>
      >>> >>>
      >>> >>>
      >>> >>>
      >>>> >>>> I'm assuming KualiMaintenanceDocumentAction / KualiInquiryAction will build up what is now the MaintenanceDocumentAuthorizations object based on all of this?
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>> >>> Yes, MaintenanceDocumentAuthorizations will contain all of the fields,
      >>> >>> and it will act as an evaluator to match the restricted property name to
      >>> >>> the actual pojo property name. For example, it may consider
      >>> >>> "document.collection.accountNumber" to be equivalent to
      >>> >>> "document.collection[10].accountNumber".
      >>> >>>
      >>> >>> * k
      >>> >>>
      >>> >>>
      >>> >>>
      >>> >>>
      >>>> >>>> Once this is done, do we want to even expose Maintainable and Inquirable getSections methods for override?
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>> >>> That may not be possible, as the getSections method needs to be on the
      >>> >>> interface for the JSP/tags to be able to render it.
      >>> >>>
      >>> >>> * it would be possible by declaring the implementations on KualiMaintainableImpl and KualiInquirableImpl final, if that's what we want to do to encourage use of the presentation controller and authorizer for the logic
      >>> >>>
      >>> >>>
      >>> >>>
      >>> >>>
      >>>> >>>> LMK what I got wrong
      >>>> >>>>
      >>>> >>>> ________________________________________
      >>>> >>>> From: Byrne, Ailish M
      >>>> >>>> Sent: Wednesday, December 17, 2008 9:43 AM
      >>>> >>>> To: Warren Liang
      >>>> >>>> Subject: RE: ok...
      >>>> >>>>
      >>>> >>>> I do see a need for folks to hide / display only sections based on non-user logic. I see that happening in VendorMaintainableImpl. But, I think you're sayng below you would give them a hook for that via a getMaintenanceDocumentAuthorizations method in the presentation controller (and of course i hate the fact that if the PC is dealing with it it has the word auth in the name). I just looked back at all our getSections user cases - in Inquirables and Maintainables. We have a few where we're hiding based on state of the BO and a few where we're hiding based on user permissions. And, that is all we are doing in the getSections methods. Here's a lot of miscellaneous thoughts.
      >>>> >>>>
      >>>> >>>> 1. Modify the View Inquiry or Maintenance Document Field(s) and Modify Maintenance Document Field(s) to accept a section id as an alternate to a property name. Add the ability to flag collections and sections in the dd as secure - only hidden and read only pertain in this case - no masking.
      >>>> >>>>
      >>>> >>>> 2. Allow them to implement 4 methods in their maintenance document authorizers - getPotentiallyReadOnlyCollectionPropertyNames... etc. - seems very yucky to me - and what's the equivalent for inquiries??
      >>>> >>>>
      >>>> >>>> 3. Have a getMaintenanceDocumentAuthorizations in the MDPC and the MDA. - I think this is what you're suggesting
      >>>> >>>> a. Add readOnlySections, securePotentiallyHiddenSections, securePotentiallyReadOnlySections, readOnlyCollection, securePotentiallyHiddenCollections, securePotentiallyReadOnlyCollections. Then, we evaluate once they've had the opportunity to modify the fields, sections and collections fields in the MDPC and the section and collections fields in the MDA (only section and collection here, since fields would have been handled in the dd). But, what about inquiries?
      >>>> >>>> b. Make them check the perms themselves when collections or sections are secure, and just have the readOnlySections and hiddenSections lists on the MaintenanceDocumentAuthorizations. This would be the only place we're not handling a template for them. But, we could add helper methods to check the template. And, we wouldn't need the secure sections part for any of our use cases cause we're just hiding sections when we hide the contained collection.
      >>>> >>>>
      >>>> >>>> There are permutations of the above, and I'm still not sure what all of this means for inquiries. And, none of this is complete. Just brainstorming and trying to wrap my head around things See the pink sections on this page for info on the stuff I think we need to get figured out / there for them as soon as possible
      >>>> >>>> https://test.kuali.org/confluence/display/KULKFS/KIM+Impacts
      >>>> >>>>
      >>>> >>>> Thanks,
      >>>> >>>> A
      >>>> >>>>
      >>>> >>>> ________________________________________
      >>>> >>>> From: Warren Liang [wliang@uci.edu]
      >>>> >>>> Sent: Wednesday, December 17, 2008 3:57 AM
      >>>> >>>> To: Byrne, Ailish M
      >>>> >>>> Subject: Re: ok...
      >>>> >>>>
      >>>> >>>> The order is as follows (earliest first):
      >>>> >>>>
      >>>> >>>> 1) MaintenanceDocumentAuthorizer.getFieldAuthorizations (to be renamed
      >>>> >>>> to getMaintenanceDocumentAuthorizations as they control both field-level
      >>>> >>>> auths and section-level auths)
      >>>> >>>> I envision
      >>>> >>>> MaintenanceDocumentAuthorizer.getFieldAuthorizations/getMaintenanceDocumentAuthorizations
      >>>> >>>> (and its MaintDocPresentationController counterpart) as returning all of
      >>>> >>>> the information on how to render a field on the MD
      >>>> >>>> (mask/readonly/hidden) and whether to remove a section.
      >>>> >>>>
      >>>> >>>> 2) Maintainable.getSections (indirectly invoked by JSP)
      >>>> >>>> Thus, Maintainable.getSections would utilize the information returned by
      >>>> >>>> MaintenanceDocumentAuthorizer.getFieldAuthorizations/getMaintenanceDocumentAuthorizations
      >>>> >>>> to construct the fields/sections.
      >>>> >>>>
      >>>> >>>> ----
      >>>> >>>> MaintenanceDocumentPresentationController.getReadOnlyFields does not
      >>>> >>>> appear to be called by the framework. However, I would imagine that if
      >>>> >>>> this method were used, it would be called within
      >>>> >>>> MaintenanceDocumentAuthorizer.getFieldAuthorizations as a method in a
      >>>> >>>> utility class, not as a customization point. What getReadOnlyFields
      >>>> >>>> should do is return information about all MaintainableFieldDefinitions
      >>>> >>>> that have unconditionallyReadOnly set to true. There would be no reason
      >>>> >>>> to make the logic customizable.
      >>>> >>>>
      >>>> >>>> MaintenanceDocumentAuthorizer.getDocumentActions would be responsible
      >>>> >>>> for controlling which buttons get displayed at the bottom of the page
      >>>> >>>> (e.g. submit/save/approve), and whether the lookup is allowed to render
      >>>> >>>> a link to the MD.
      >>>> >>>>
      >>>> >>>> Does this seem like a clear and understandable separation of concerns
      >>>> >>>> for you?
      >>>> >>>>
      >>>> >>>> Thanks,
      >>>> >>>>
      >>>> >>>> Warren
      >>>> >>>>
      >>>> >>>>
      >>>> >>>> Byrne, Ailish M wrote:
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>>> >>>>
      >>>>> >>>>> here's what i'm struggling with re: maint docs - maintainables vs.
      >>>>> >>>>> presentation controllers vs. authorizers...
      >>>>> >>>>>
      >>>>> >>>>> where are people supposed to do what re:
      >>>>> >>>>> 1. Maintainable.getSections
      >>>>> >>>>> 2. MaintenanceDocumentPresentationController.getReadOnlyFields
      >>>>> >>>>> 3. MaintenanceDocumentAuthorizer.getFieldAuthorizations - which is a
      >>>>> >>>>> horrible name by the way
      >>>>> >>>>>
      >>>>> >>>>> are they listed in the order they get called?
      >>>>> >>>>>
      >>>>> >>>>> - 1, 2, and 3 allow control over field level
      >>>>> >>>>> - 1 and 3 allow control over sections - thereby stating the assumption
      >>>>> >>>>> i guess that we would never hide a section based on anything but
      >>>>> >>>>> permissions
      >>>>> >>>>> - interesting that 2 assumes we would not want to hide any fields or
      >>>>> >>>>> do anything section related
      >>>>> >>>>> - what does 1 allow that 2 and 3 don't - direct field manipulation?
      >>>>> >>>>> can they modify the attribute security too
      >>>>> >>>>>
      >>>>> >>>>>
      >>>>> >>>>>
      >>>>> >>>>>
      >>>>> >>>>>
      > >
      > >

        Issue Links

          Activity

          There are no comments yet on this issue.

            People

            • Assignee:
              Unassigned
              Reporter:
              Ailish Byrne
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Structure Helper Panel