Skip to content

CFM-105_Oauth2Server_MVC #310

Merged
merged 1 commit into from
Apr 28, 2025
Merged

Conversation

Ioannis
Copy link
Contributor

@Ioannis Ioannis commented Apr 28, 2025

No description provided.

Renane callback uri field to redirect uri

Fetch access token
@Ioannis Ioannis merged commit 576f8c2 into COmanage:develop Apr 28, 2025
@Ioannis Ioannis deleted the CFM-105_Oauth2Server_MVC branch April 28, 2025 12:19
* @param array $context Validation context
* @return mixed True if $value validates, or an error string otherwise
*/
public function validateNotBlank(mixed $value, array $context): mixed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating a new validation rule (it's not obvious from the name what the relation is to Cake's notEmptyString), can we update the validation definitions to use registerStringValidation?

eg, why can't we just do something like this:

$this->registerStringValidation($validator, $schema, 'scope', false);

Copy link
Contributor Author

@Ioannis Ioannis May 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benno, the method is an extraction of the following logic: validateNotBlank

// We require at least one non-whitespace character (CO-1551)
if (!preg_match('/\S/', $value)) {
    return __d('error', 'input.blank');
}

This is found in the custom validateInput function currently used in the COmanage v5 framework (built on CakePHP 4). That function also marks strings containing the characters <> as invalid. What if we only want to check for non-blank input—i.e., ensure there’s at least one non-whitespace character—without rejecting strings simply because they contain <>? Would using alone be more appropriate for this specific case? validateNotBlank

I cross referenced CAKEPHP2 implementation for this one.

Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants