2012-09-19 17 views
11

En mi Controlador antes de que se modifique un Modelo (actualizado o eliminado) Estoy intentando verificar que el Usuario que realiza la acción posee realmente el objeto que está intentando modificar.Comprobación de propiedad del modelo

Actualmente estoy haciendo esto en el nivel de método y parece un poco redundante.

[HttpPost] 
public ActionResult Edit(Notebook notebook) 
{ 
    if (notebook.UserProfileId != WebSecurity.CurrentUserId) { return HttpNotFound(); } 

    if (ModelState.IsValid) 
    { 
     db.Entry(notebook).State = EntityState.Modified; 
     db.SaveChanges(); 
     return RedirectToAction("Index"); 
    } 
    return View(notebook); 
} 

¿Existe una forma genérica de hacerlo que pueda ser reutilizable en varios modelos?

¿Es posible hacer esto con un ActionFilter?

Respuesta

2

Puedo ver un problema con lo que tiene: depende de la entrada del usuario para realizar la comprobación de seguridad.

Tenga en cuenta su código

if (notebook.UserProfileId != WebSecurity.CurrentUserId) 

Notebook ha venido de modelo de unión. Entonces UserProfileId proviene del enlace del modelo. Y puedes fingir felizmente que, por ejemplo, uso el TamperData de Firefox para cambiar el valor del UserProfileId oculto para que coincida con mi inicio de sesión y fuera de allí.

Lo que termino haciendo (en un servicio, en lugar del controlador) es en una publicación retirando el registro de la base de datos basado en el ID único pasado (Edit/2 por ejemplo usaría 2), y luego revisando User.Identity.Nombre (bueno, el parámetro de identidad pasado) contra el campo de propietario actual que tengo en mi registro de base de datos devuelto.

Como retiro de la base de datos (repositorio, lo que sea) un atributo no va a funcionar para esto, y no estoy seguro de que pueda ser lo suficientemente genérico en el enfoque de un atributo de todos modos.

+0

¿No se agregaría [Bind (Exclude = "UserProfileID"]] para evitar que el enlace del modelo lo cambie? Idealmente, me gustaría devolver un ViewModel que no contenga ningún campo ID y solo tener los almacenados en TempData o algo así (no del todo seguro) ... – mezmi

+0

Sí, pero ¿de dónde vendría? :) – blowdart

+0

Sí, creo que realmente no lo había pensado. Como dije, me gustaría devolver un ViewModel que no contenga ningún campo de identificación. Simplemente no sé dónde almacenar ese ID temporalmente en MVC entre llamadas, además de usar Session>:) .... De esa manera pude devuelva mi POCO al Controlador y use AutoMapper o algo para hidratar ViewModel. – mezmi

3

El filtro suena como un enfoque correcto, pero es algo limitado. Sería bueno si pudiera tener un filtro de la siguiente manera:

[RequireOwnership<Notebook>(n => n.UserProfileId)] 

... pero Attributes están limitados en el que se permite tipos de datos, y no creo que los genéricos se permite tampoco. Por lo que podría tener un atributo [RequireOwnership] que funciona mediante la inspección del modelo utilizando la reflexión, o se puede crear un validador propio lugar, donde el modelo se parece a esto:

public class Notebook 
{ 
    [MatchesCurrentUserId] 
    public int UserProfileId { get; set; } 
} 

Entonces su cheque ModelState.IsValid debería ser suficiente.

Editar:

Otra opción se me ocurrió. Puede usar un filtro junto con un atributo en su modelo (no tiene que ser un ValidationAttribute). El filtro podría inspeccionar sus modelos de solicitud y verificar las propiedades con [MatchesCurrentUserId], en comparación con la identificación de usuario actual.

+0

He estado yendo y viniendo con la idea de usar ValidationAttribute. Me gusta la idea pero, al mismo tiempo, siento que debería devolver Http 401 No autorizado frente a un error de validación. ¿Alguna idea? – mezmi

+0

La respuesta no autorizada es definitivamente mejor. Puede inspeccionar los resultados de validación y determinar si se debe devolver una respuesta 401. O tal vez el atributo de validación podría arrojar una excepción que finalmente se traduciría en una respuesta 401 (como quizás lanzar una 'HttpException') – Jacob

+0

Ahhh, pero si tiene que inspeccionarlos, ¿está escribiendo nuevamente más código en el controlador? – jocull

1

Cuando he hecho cosas como esta en el pasado, realmente no ha sido mucho mejor. Para nuestros proyectos, tendríamos un método que aceptaría un objeto Notebook y lo verificaría contra el usuario actualmente conectado.

Puede sobrecargar este método con todos sus tipos de objeto diferentes y utilizar un método coherente para verificar el acceso. Lo siento, esa es la mejor manera que conozco.

[HttpPost] 
public ActionResult Edit(Notebook notebook) 
{ 
    if(!SessionUser.LoggedInUser.CheckAccess(notebook)) 
     return HttpNotFound(); 

    //Other code... 
} 

P.S. SessionUser es una clase personalizada que básicamente teníamos solo para administrar a quien haya iniciado sesión en ese momento. Podrías escribir algo similar, pero no esperes que esté en .NET por defecto.

5

enfoque Un filtro podría ser:

public class VerifyOwnership : IActionFilter 
{ 
    public void OnActionExecuting(ActionExecutingContext filterContext) 
    { 
     foreach(var parameter in filterContext.ActionParameters) 
     { 
      var owned = paramter.Value as IHaveAnOwner; 
      if(owned != null) 
      {      
       if(owned.OwnerId != WebSecurity.CurrentUserId) 
       { 
        // ... not found or access denied 
       } 
      } 
     } 
    } 

    public void OnActionExecuted(ActionExecutedContext filterContext) 
    { 

    } 
} 

que asume como modelos Notebook implementar una interfaz específica.

public interface IHaveAnOwner 
{ 
    int OwnerId { get; set; } 
} 

Blowdart tiene un buen punto que un usuario podría manipular el OwnerId en una publicación. Estoy seguro de que también podrían alterar su boleto de autenticación, pero tendrían que conocer el boleto del otro usuario y alterar ambos para lograr que los ID coincidan con otro usuario, creo.

+0

Esa es una buena solución: tendría que codificar los objetos recién creados y algunas otras capturas, estoy seguro – jocull

Cuestiones relacionadas