isAuthorized not working for WaiSubsite

[mchaver]

I'm trying to add a servant API behind Yesod that requires authentication as provided by the main Yesod app using this example project.

https://github.com/parsonsmatt/yo-dawg

/api ServantPersistentR WaiSubsite getServantPersistent

instance Yesod App where
    ....
    isAuthorized (AuthR _) _ = return Authorized
    isAuthorized (ServantPersistentR _) _ = return $ Unauthorized "unauthorized"
    isAuthorized _ _              = return AuthenticationRequired

However, using curl or a browser I can still access the servant routes.

[snoyberg]

Please see #1286

[mchaver]

@snoyberg
I did see that but I am not quite sure what I need to add to get it to work. An instance of YesodSubDispatch WaiSubsite (HandlerT master IO)?

Edit: hmm, I guess that musn't be it since that instance is defined in Yesod.Core.Class.Dispatch.

[snoyberg]

As a stop-gap measure, how about a newtype wrapper alternative to WaiSubsite along the lines of WaiSubsiteWithAuth that uses the instance mentioned? If that works, maybe we should just add it to yesod-core and document both the original newtype and the new one appropriately.

…

On Thu, May 11, 2017 at 5:37 PM, James Haver II ***@***.***> wrote: @snoyberg <https://github.com/snoyberg> I did see that but I am not quite sure what I need to add to get it to work. An instance of YesodSubDispatch WaiSubsite (HandlerT master IO)? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1393 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADBB0F_9Elzzt2Wg_Emv_MFFK7pzSavks5r4x02gaJpZM4NX3Aj> .

[mchaver]

that sounds like a reasonable solution

[mchaver]

I figured it out. I'll make a PR

[snoyberg]

Awesome

…

On Thu, May 11, 2017 at 6:47 PM, James Haver II ***@***.***> wrote: I figured it out. I'll make a PR — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1393 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADBB8ALOWmuyRebhBtlLJzzWNBTIr6Cks5r4y1_gaJpZM4NX3Aj> .

[mchaver]

#1394

[jezen]

This appears to be resolved by the merged PR, so I will close this issue.