Skip to content

Conversation

@ADmad
Copy link
Member

@ADmad ADmad commented Jan 4, 2021

Remove dev centric error message. Since UnauthenticatedException now extends HttpException this is the message that gets shown to end user which is not desirable. Also made the error message configurable.

ADmad added 2 commits January 4, 2021 18:21
Since UnauthenticatedException now extends HttpException this is the message
that gets shown to end user which is not desirable.
@ADmad ADmad added this to the 2.next milestone Jan 4, 2021
throw new UnauthenticatedException(
'No identity found. You can skip this check by configuring `requireIdentity` to be `false`.'
);
throw new UnauthenticatedException($this->getConfig('unauthenticatedMessage', ''));
Copy link
Member

Choose a reason for hiding this comment

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

Should we have a default message? Something like 'Authentication is required'?

Copy link
Member Author

@ADmad ADmad Jan 4, 2021

Choose a reason for hiding this comment

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

The constructor already sets one:

if (!$message) {
$message = 'Authentication is required to continue';
}

Copy link
Member

Choose a reason for hiding this comment

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

Good job past us.

@markstory markstory merged commit dbe7cde into 2.next Jan 4, 2021
@markstory markstory deleted the exception-message branch January 4, 2021 21:52
@othercorey
Copy link
Member

Should we allow the exception message to be empty instead of forcing the default in the constructor?

@markstory
Copy link
Member

I don't think an empty message is useful behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants