D18845: authority: add support for passing details to polkit

ngraham added a comment. Landed and changed the comment. @chinmoyr, what's next for T8075: Fix security issues with KAuth support in KIO ? REPOSITORY R563 Polkit-1 Qt Library REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner,

D18845: authority: add support for passing details to polkit

This revision was automatically updated to reflect the committed changes. Closed by commit R563:f26ada834aea: authority: add support for passing details to polkit (authored by mgerstner, committed by ngraham). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D18845?vs=54931=55297#toc

D18845: authority: add support for passing details to polkit

davidedmundson accepted this revision. REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks, chinmoyr, fvogt, bruns, davidedmundson Cc: elvisangelaccio, ngraham, mati865, kde-frameworks-devel

D18845: authority: add support for passing details to polkit

ngraham added a comment. So what's next for this? Can this land now or do we need to change the comment first? REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks, chinmoyr, fvogt, bruns Cc: elvisangelaccio, ngraham, mati865, kde-frameworks-devel

D18845: authority: add support for passing details to polkit

elvisangelaccio added inline comments. INLINE COMMENTS > polkitqt1-authority.h:226 > + */ > +// TODO KF6: merge with checkAuthorization > +void checkAuthorizationWithDetails( @bruns The `polkit-qt-1` repository is not a framework, so "KF6" could be misleading here. What about

D18845: authority: add support for passing details to polkit

mgerstner added a comment. In D18845#439430 , @ngraham wrote: > @mgerstner can you provide your email address so we can land this patch with correct authorship information? It's matthias.gerst...@suse.com. REVISION DETAIL

D18845: authority: add support for passing details to polkit

ngraham added a comment. @mgerstner can you provide your email address so we can land this patch with correct authorship information? REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks, chinmoyr, fvogt, bruns Cc: ngraham, mati865, kde-frameworks-devel

D18845: authority: add support for passing details to polkit

bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. Thanks REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks, chinmoyr, fvogt, bruns Cc: ngraham, mati865, kde-frameworks-devel

D18845: authority: add support for passing details to polkit

mgerstner updated this revision to Diff 54931. mgerstner added a comment. Now using `constData()` as suggest by chinmoyr. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18845?vs=51753=54931 REVISION DETAIL https://phabricator.kde.org/D18845 AFFECTED FILES

D18845: authority: add support for passing details to polkit

mgerstner added inline comments. INLINE COMMENTS > chinmoyr wrote in polkitqt1-authority.cpp:336 > Nitpick; constData() because the API seems to take const gchar* Strictly spoken it already returns `const gchar*`, since `toUtf8()` returns a `const QByteArray` temporary object and thus the

D18845: authority: add support for passing details to polkit

bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. Can you address the last nitpick from @chinmoyr ? REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks, chinmoyr, fvogt, bruns Cc: ngraham, mati865,

D18845: authority: add support for passing details to polkit

ngraham added a comment. Three LGTMs and +1 are pretty good :) REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks, chinmoyr, fvogt, bruns Cc: ngraham, mati865, kde-frameworks-devel

D18845: authority: add support for passing details to polkit

chinmoyr added a comment. I would like this patch to land so +1 from me. But I am not familiar with this code so I can't give any meaningful feedback. Sorry. INLINE COMMENTS > polkitqt1-authority.cpp:336 > + > +polkit_details_insert(ret, key.toUtf8().data(), >

D18845: authority: add support for passing details to polkit

fvogt added a comment. LGTM to me as well - I'm not familiar with this code at all though. REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks, chinmoyr, fvogt, bruns Cc: mati865, kde-frameworks-devel

D18845: authority: add support for passing details to polkit

bruns added a comment. LGTM - @fvogt , @chinmoyr ? REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks, chinmoyr, fvogt, bruns Cc: mati865, kde-frameworks-devel

D18845: authority: add support for passing details to polkit

mgerstner added a comment. In D18845#416305 , @bruns wrote: > Does this solve part of T8075 ? Part of a part it seems. I am currently working towards T10480 . To

D18845: authority: add support for passing details to polkit

bruns added a comment. Does this solve part of T8075 ? REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks, chinmoyr, fvogt, bruns Cc: kde-frameworks-devel

D18845: authority: add support for passing details to polkit

mgerstner added inline comments. INLINE COMMENTS > bruns wrote in polkitqt1-authority.cpp:328 > nullptr I didn't want to mix styles in the source files. It's adjusted now. REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks, chinmoyr, fvogt, bruns Cc:

D18845: authority: add support for passing details to polkit

mgerstner updated this revision to Diff 51753. mgerstner added a comment. Incorporated review comments: replaced `NULL` by `nullptr`, removed some extra whitespace within parantheses, added KF6 TODO. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18845?vs=51161=51753 REVISION

D18845: authority: add support for passing details to polkit

bruns added inline comments. INLINE COMMENTS > polkitqt1-authority.cpp:130 > + * The returned pointer needs to be freed via g_object_unref when no > + * longer needed. Returns NULL if details is empty. > + */ nullptr > polkitqt1-authority.cpp:328 > +if ( details.empty() ) > +

D18845: authority: add support for passing details to polkit

ngraham added reviewers: chinmoyr, fvogt, bruns. REPOSITORY R563 Polkit-1 Qt Library REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks, chinmoyr, fvogt, bruns Cc: kde-frameworks-devel

D18845: authority: add support for passing details to polkit

lbeltrame added a reviewer: Frameworks. lbeltrame added a subscriber: kde-frameworks-devel. REPOSITORY R563 Polkit-1 Qt Library REVISION DETAIL https://phabricator.kde.org/D18845 To: mgerstner, #frameworks Cc: kde-frameworks-devel