Skip to content

Conversation

@albe
Copy link

@albe albe commented Apr 14, 2021

  • Add middleware
  • Fix/improve communication from Provider to Middleware for redirect
  • Remove old component
  • Update Settings

{
$response = $next->handle($request);
// TODO: Find a better solution to communicate from TokenProvider to this middleware
$redirectTarget = $response->getHeaderLine(static::class . '.redirect');
Copy link
Author

Choose a reason for hiding this comment

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

See neos/flow-development-collection#2019 (comment)

Though we do not even have access to the ActionResponse in the TokenProvider, so this might become a bit harder to achieve than anticipated. If anyone has a good idea, just shoot.
I'm still thinking if it wouldn't make more sense to use the entryPoint configuration for the login redirect and use a middleware that checks for the setup redirect.

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

@albe @johannessteu
I think it could work like that:
master...gerdemann:Neos7

Copy link
Collaborator

Choose a reason for hiding this comment

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

using exceptions for control flow is a bit quirky because of the overhead of throwing/catching exceptions (i.e. they have to create a full stack trace).
But in this case I also think that it's the nicer solution – and one top-level exception shouldn't do any harm

Choose a reason for hiding this comment

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

@albe Do you want to adapt your PR or should I create a new one? 😄

Copy link
Collaborator

@bwaidelich bwaidelich Apr 14, 2022

Choose a reason for hiding this comment

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

@gerdemann btw I think your solution could be simplified to

    public function process(ServerRequestInterface $request, RequestHandlerInterface $next): ResponseInterface
    {
        try {
            $response = $next->handle($request);
        } catch (\Exception $exception) {
            if ($exception instanceof SecondFactorLoginException || $exception->getPrevious() instanceof SecondFactorLoginException) {
                return $this->redirectToLogin($request);
            } elseif ($exception instanceof SecondFactorSetupException || $exception->getPrevious() instanceof SecondFactorSetupException) {
                return $this->redirectToSetup($request);
            } else {
                throw $exception;
            }
        }
        return $response;
    }

Choose a reason for hiding this comment

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

Thank you, I have now adjusted this and created a PR. It doesn't matter to me whether this PR is adjusted or mine is merged. 😃
#8

@albe
Copy link
Author

albe commented Apr 14, 2022

Closing in favor of #8

@albe albe closed this Apr 14, 2022
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.

3 participants