Thanks Nathan! I feel like this use-case is so specific that it should probably go into a custom authproc-filter.

Thanks Nathan! I feel like this use-case is so specific that it should probably go into a custom authproc-filter.

Yeah, that was my inital thoughts too. My original design was this:

    'authproc' => [
        50 => [
            'class' => 'core:AttributeConditionalAdd',
            'conditions' => [
                'attrExists' => [
                    'eduPersonAffiliation',
                ],
                'attrExistsRegex' => [
                    '/^person.*$/',
                ],
                'attrValueIs' => [
                    'departmentName' => 'Physics',
                ],
                'attrValueIsRegex' => [
                    'employeeType' => '/^staff$/',
                ],
            ],
            'attributes' => [
                'isInternal' => ['true'],
            ],
        ],
    ],

But the base case is just:

'authproc' => [
        50 => [
            'class' => 'core:AttributeConditionalAdd',
            'conditions' => [], // empty array should be default, so we shouldn't need to specify it
            'attributes' => [
                'attributeToAdd' => ['true'],
            ],
        ],
    ],

Which is just core:AttributeAdd (an empty conditions array evaluates to true, and is the default). So retrofitting conditionals to core:AttributeAdd was what I went with.

I'm happy to submit a PR for core:AttributeConditionalAdd if you'd consider it for merging. I don't think conditional adding of attributes would be that unusual to do. Also happy to take feedback on the above design.

Another thing I considered doing was just using %precondition filters, but I could see that that method would quickly become an unmaintainable mess of hard to test PHP code.

Thanks Tim. I appreciate the feedback.

I'm curious what this looks like with the existing %precondition for conditionally running the authproc. Or these conditions too conplex/awkward to fit into that framework?

I'm curious what this looks like with the existing %precondition for conditionally running the authproc. Or these conditions too conplex/awkward to fit into that framework?

Honestly, I haven't tried. The use of %precondition I see as a last resort, and providing something more declarative (and easier to test) preferable.