On 09/05/18 23:24, Colin Watson wrote: > I don't understand the principle here. Can you explain why it's any > more of a layering violation for a BPB issuer to grant permission to > access its associated LFAs than it is for a CodeImportJob issuer to > grant permission to access its associated GitRepositories?
It was less obviously wrong there because the GitRepository authorisation code already touched CodeImport and they were part of the same Launchpad application. But in this case we have the librarian touching complex bits of the Soyuz schema, which is ugly from a code structure perspective and also means we have to be very careful around synchronising schema changes and librarian deployments. librarian today only examines non-librarian tables when GC introspects the schema to work out which FKs there are to LFA.id, and that doesn't seem like an architectural separation that we should abandon lightly. > In general, my intent in designing the IMacaroonIssuer interface was > that various subsystems could grant the specific permissions they > needed, and the verifying code could then use the > getUtility(IMacaroonIssuer, token.identifier) idiom to get the > appropriate verifier without having to have specific knowledge of > what the verifier in question does. Apart from the security.cfg > changes, nothing about the librarian changes here is at all specific > to BPBs, and it would be perfectly possible to write a different > verifier that grants access based on some other criteria; > furthermore, if (hypothetically) a BPB needed access to some other > private resource, the same macaroon could be used for both by > generalising the verifier. I agree that sharing macaroons would be ideal, but having the librarian itself validate macaroons that may require arbitrarily broad and complex database logic (particularly given the librarian is a Twisted app which really shouldn't be doing synchronous DB calls at all) doesn't seem right. -- https://code.launchpad.net/~cjwatson/launchpad/librarian-accept-macaroon/+merge/345079 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
