-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Managed ACL: Add AccessRestrictionList support #34932
Conversation
Review changes with SemanticDiff. Analyzed 1 of 40 files. File Information
|
PR #34932: Size comparison from e994cbf to 5501b22 Increases above 0.2%:
Full report (80 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
first pass feedback
PR #34932: Size comparison from ddf44dc to 5e96425 Increases above 0.2%:
Full report (10 builds for cc32xx, nrfconnect, qpg, stm32, tizen)
|
PR #34932: Size comparison from ddf44dc to 668fefe Increases above 0.2%:
Full report (68 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
PR #34932: Size comparison from 62139bf to 9ed7dcc Increases above 0.2%:
Full report (7 builds for cc32xx, qpg, stm32, tizen)
|
PR #34932: Size comparison from 62139bf to c16e6e2 Increases above 0.2%:
Full report (5 builds for cc32xx, stm32, tizen)
|
PR #34932: Size comparison from 62139bf to 3bba771 Increases above 0.2%:
Full report (46 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, nrfconnect, nxp, psoc6, qpg, stm32, tizen)
|
PR #34932: Size comparison from 62139bf to 9766e30 Increases above 0.2%:
Full report (82 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
PR #34932: Size comparison from 128c37a to febc716 Increases above 0.2%:
Full report (5 builds for cc32xx, stm32, tizen)
|
PR #34932: Size comparison from 128c37a to 0739363 Increases above 0.2%:
Full report (82 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
src/app/clusters/access-control-server/access-control-server.cpp
Outdated
Show resolved
Hide resolved
- fixed return type for some command failures - enhanced unit tests
PR #34932: Size comparison from acba7f8 to d27dfb6 Increases above 0.2%:
Full report (77 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
PR #34932: Size comparison from 7ddba36 to cbcd55a Increases above 0.2%:
Full report (77 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Clearing since outstanding issues have been cleared. Boris is on PTO and team is trying to hit deadline to merge today. If anything is missed, can follow up in a subsequent PT.
Head branch was pushed to by a user without write access
PR #34932: Size comparison from d185778 to 31a7ddd Increases above 0.2%:
Full report (41 builds for bl602, bl702, bl702l, cyw30739, linux, nrfconnect, nxp, psoc6, qpg, stm32, tizen)
|
PR #34932: Size comparison from d185778 to 141c555 Increases above 0.2%:
Full report (12 builds for nrfconnect, nxp, qpg, stm32, tizen)
|
src/app/clusters/access-control-server/access-control-server.cpp
Outdated
Show resolved
Hide resolved
PR #34932: Size comparison from d185778 to 3d976bf Increases above 0.2%:
Full report (77 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
* Add AccessRestrictionList support * Update src/access/AccessConfig.h Co-authored-by: C Freeman <cecille@google.com> * Reworked data manipulators and other cleanup * Fixed encode/decode so reading CommissioningARL and Arl attributes work * Reworked ARL storage Previously ARL related data was persisted in KVS. This has been removed and now the responsibility for managing/maintaining the related data (CommissioningARL and ARL attributes) is up to the app to set on AccessRestrictionProvider class. * Review fixes cleanup ArlEncoder interface. return error to client if arl review request fails return token to client in FabricRestrictionReviewUpdate * Fixed GetEntries vector pointer arg * Updated core restriction logic/integration * Restyled by clang-format * fixed include check for renamed AccessRestrictionProvider.h file * M-ACL updates - refactored AccessControl::Check into CheckACL and CheckARL - added placeholders for the upcoming CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL - extracted ARL exception processing to standalone class for better testing * Add plumbing for subject descriptor IsCommissioning field - Make session manager update that state on a message-per-message basis - Add tests Missing test: MRP test against a not-yet-committed fabric over CASE showing that IsCommissioning is true. * Fix crash * Use new IsCommissioning in ARL check * Updates for review comments * restyled * Review updates - fixed return type for some command failures - enhanced unit tests * restyled * Updated ARL tests per review comments * work around nuttx and jsoncpp contention * Review comments and nuttx build failure fix attempt * review updates --------- Co-authored-by: C Freeman <cecille@google.com> Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: tennessee.carmelveilleux@gmail.com <tennessee.carmelveilleux@gmail.com>
* Add AccessRestrictionList support * Update src/access/AccessConfig.h Co-authored-by: C Freeman <cecille@google.com> * Reworked data manipulators and other cleanup * Fixed encode/decode so reading CommissioningARL and Arl attributes work * Reworked ARL storage Previously ARL related data was persisted in KVS. This has been removed and now the responsibility for managing/maintaining the related data (CommissioningARL and ARL attributes) is up to the app to set on AccessRestrictionProvider class. * Review fixes cleanup ArlEncoder interface. return error to client if arl review request fails return token to client in FabricRestrictionReviewUpdate * Fixed GetEntries vector pointer arg * Updated core restriction logic/integration * Restyled by clang-format * fixed include check for renamed AccessRestrictionProvider.h file * M-ACL updates - refactored AccessControl::Check into CheckACL and CheckARL - added placeholders for the upcoming CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL - extracted ARL exception processing to standalone class for better testing * Add plumbing for subject descriptor IsCommissioning field - Make session manager update that state on a message-per-message basis - Add tests Missing test: MRP test against a not-yet-committed fabric over CASE showing that IsCommissioning is true. * Fix crash * Use new IsCommissioning in ARL check * Updates for review comments * restyled * Review updates - fixed return type for some command failures - enhanced unit tests * restyled * Updated ARL tests per review comments * work around nuttx and jsoncpp contention * Review comments and nuttx build failure fix attempt * review updates --------- Co-authored-by: C Freeman <cecille@google.com> Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: tennessee.carmelveilleux@gmail.com <tennessee.carmelveilleux@gmail.com>
Add Access Restriction support