Identity OAuth Implementación con Owin -- # campo con asp.net campo con mvc campo con oauth campo con owin camp codereview Relacionados El problema

Identity OAuth implementation with Owin


7
vote

problema

Español

Estoy construyendo una aplicación MVC 5 usando el marco de entidad de código 6 y un modelo de identidad ASP.NET personalizado para la autenticación.

Solo estaré autentificando contra los proveedores externos de OAURH (Google, Facebook, etc.) y persistiré en la información de inicio de sesión y reclamaciones en entidades de EF de código personalizado.

Dado que esta es la primera vez que trabajé con el modelo de identidad ASP.NET y Owin, quiero asegurarme de que no he hecho ningún error importante en el código que tenga hasta ahora, lo que funciona y registra un nuevo El usuario, guarda sus reclamaciones y persiste su inicio de sesión usando una cookie.

Intentaré mantener el código lo más pequeño posible mientras aún muestra todos los detalles relevantes. Estoy usando AutoFAC para inyección de dependencia.

user.cs - Implementación de IdentityUser

  public class User : IdentityUser<string, UserLogin, UserRole, UserClaim> {     public User()     {         // without this Id would be NULL when I create a new User         base.Id = Guid.NewGuid().ToString();     }      public User(string userName)         : this()     {         base.UserName = userName;     }      .... other properties }   

userlogin , userrole y userClaim son simplemente clases que implementan su clase de base 99887776665544333

dbcontext.cs - DBContext implementando IdentityDbContext con las clases anteriores

  public class DbContext : IdentityDbContext<User, Role, string, UserLogin, UserRole, UserClaim> {}   

userservice.cs - Implementación de UserStore

  public class UserService : UserStore<User, Role, string, UserLogin, UserRole, UserClaim> {}   

roleservice.cs - implementación de rolestore

  public class RoleService : RoleStore<Role, string, UserRole> {}   

ApplicationClaimsIdentityFactory.cs - Reimplementación de reclamacionesIdentidades de atención. Necesario porque estaba obteniendo reclamaciones duplicadas agregadas porque los reclamos de nombre y nombre de identificación se guardan en la base de datos y se reinicia en el inicio de sesión.

  public class ApplicationClaimsIdentityFactory : ClaimsIdentityFactory<User, string> {     // identical to normal ClaimsIdentityFactory until:         if (manager.SupportsUserClaim)         {             var userClaims = await manager.GetClaimsAsync(user.Id).WithCurrentCulture();             foreach (var userClaim in userClaims)             {                 if (!id.HasClaim(userClaim.Type, userClaim.Value))                 {                     id.AddClaim(userClaim);                 }             }         }         return id;     } }   

applicationusermanager.cs - Implementación de Usermanager Especificando para usar reclamaciones personalizadasIntityFactory

  public class ApplicationUserManager : UserManager<User, string> {     public ApplicationUserManager(UserStore<User, Role, string, UserLogin, UserRole, UserClaim> store) : base(store)     {         this.ClaimsIdentityFactory = new ApplicationClaimsIdentityFactory();     }      public static ApplicationUserManager Create(IdentityFactoryOptions<ApplicationUserManager>options, IOwinContext context)     {         var manager = new ApplicationUserManager(new UserService(context.Get<DbContext>()));         return manager;     } }   

solicitunsigninmanager.cs - Implementación de SigninManager con nuevas entidades

  IdentityUser0  

startup.auth.cs

  IdentityUser1  

accountcontroller.cs

  IdentityUser2  

owinhelper.cs

  IdentityUser3  

Creo que eso es todo lo que es relevante. Como dije, estoy buscando una revisión de mi identidad y la implementación de OAURH, no la calidad general del código. He eliminado muchos cheques nulos y por igual para intentar acortar un poco el código.

Por favor, hágamelo saber si hay algo que cree que podría haber perdido, y cualquier comentario en mi código estoy feliz de recibir.

Original en ingles

I am building an MVC 5 application using code-first Entity Framework 6 and a customised ASP.NET Identity model for authentication.

I will only be authenticating against external OAuth providers (Google, Facebook etc) and persisting the login and claims information in custom code-first EF entities.

Since this is the first time I have worked with the ASP.NET Identity model and Owin, I want to make sure I haven't made any major errors in the code I have so far, which does work and registers a new user, saves their claims and persists their login using a cookie.

I will try to keep the code as small as possible while still showing all relevant details. I am using AutoFac for dependency injection.

User.cs - Implementation of IdentityUser

public class User : IdentityUser<string, UserLogin, UserRole, UserClaim> {     public User()     {         // without this Id would be NULL when I create a new User         base.Id = Guid.NewGuid().ToString();     }      public User(string userName)         : this()     {         base.UserName = userName;     }      .... other properties } 

UserLogin, UserRole and UserClaim are simply classes that implement their relevant IdentityUserX base class.

DbContext.cs - Standard DbContext implementing IdentityDbContext with above classes

public class DbContext : IdentityDbContext<User, Role, string, UserLogin, UserRole, UserClaim> {} 

UserService.cs - Implementation of UserStore

public class UserService : UserStore<User, Role, string, UserLogin, UserRole, UserClaim> {} 

RoleService.cs - Implementation of RoleStore

public class RoleService : RoleStore<Role, string, UserRole> {} 

ApplicationClaimsIdentityFactory.cs - reimplementation of ClaimsIdentityFactory. Needed because I was getting duplicate claims added because the Name and NameIdentifier claims are saved into the database, and re-added at sign in.

public class ApplicationClaimsIdentityFactory : ClaimsIdentityFactory<User, string> {     // identical to normal ClaimsIdentityFactory until:         if (manager.SupportsUserClaim)         {             var userClaims = await manager.GetClaimsAsync(user.Id).WithCurrentCulture();             foreach (var userClaim in userClaims)             {                 if (!id.HasClaim(userClaim.Type, userClaim.Value))                 {                     id.AddClaim(userClaim);                 }             }         }         return id;     } } 

ApplicationUserManager.cs - Implementation of UserManager specifying to use custom ClaimsIdentityFactory

public class ApplicationUserManager : UserManager<User, string> {     public ApplicationUserManager(UserStore<User, Role, string, UserLogin, UserRole, UserClaim> store) : base(store)     {         this.ClaimsIdentityFactory = new ApplicationClaimsIdentityFactory();     }      public static ApplicationUserManager Create(IdentityFactoryOptions<ApplicationUserManager>options, IOwinContext context)     {         var manager = new ApplicationUserManager(new UserService(context.Get<DbContext>()));         return manager;     } } 

ApplicationSignInManager.cs - Implementation of SignInManager with new entities

public class ApplicationSignInManager: SignInManager<User, string> {     public ApplicationSignInManager(ApplicationUserManager userManager, IAuthenticationManager authManager)         : base(userManager, authManager)     {     }      public static ApplicationSignInManager Create(IdentityFactoryOptions<ApplicationSignInManager> options, IOwinContext context)     {         return new ApplicationSignInManager(context.GetUserManager<ApplicationUserManager>(), context.Authentication);     } } 

Startup.Auth.cs

public partial class Startup {     static Startup()     {         PublicClientId = "self";         ExternalAuthPageUrl = "Account/ExternalAuthRequestCallback";         var applicationOAuthProvider = new ApplicationOAuthProvider(PublicClientId, ExternalAuthPageUrl);         OAuthOptions = new OAuthAuthorizationServerOptions         {             Provider = applicationOAuthProvider,             RefreshTokenProvider = applicationOAuthProvider,             AuthorizeEndpointPath = new PathString("/Account/ExternalLogin"),             AccessTokenExpireTimeSpan = TimeSpan.FromDays(30),             AllowInsecureHttp = false         };     }      public static OAuthAuthorizationServerOptions OAuthOptions { get; private set; }     public static string PublicClientId { get; private set; }     public static string ExternalAuthPageUrl { get; private set; }      public void ConfigureAuth(IAppBuilder app)     {                     app.UseCookieAuthentication(new CookieAuthenticationOptions         {             AuthenticationType = DefaultAuthenticationTypes.ApplicationCookie,             LoginPath = new PathString("/Account/Login"),             Provider = new CookieAuthenticationProvider             {                 OnValidateIdentity = SecurityStampValidator.OnValidateIdentity<ApplicationUserManager, User>(                                         validateInterval: TimeSpan.FromMinutes(15),                                         regenerateIdentity: (manager, user) => user.GenerateUserIdentityAsync(manager))             },             SlidingExpiration = true,             ExpireTimeSpan = TimeSpan.FromMinutes(30)         });          app.UseExternalSignInCookie(DefaultAuthenticationTypes.ExternalCookie);          app.UseOAuthBearerTokens(OAuthOptions);          var authOptions = new GoogleOAuth2AuthenticationOptions();         authOptions.ClientId = "OAuthKey";         authOptions.ClientSecret = "OAuthSecret";         authOptions.Scope.Add("email");          app.UseGoogleAuthentication(authOptions);     } } 

AccountController.cs

[AllowAnonymous] // displays a list of available external authenticators for the user to pick public ActionResult Login(string returnUrl = "/") {     IEnumerable<AuthenticationDescription> descriptions = Request.GetOwinContext().Authentication.GetExternalAuthenticationTypes();     IList<ExternalProviderViewModel> providers = new List<ExternalProviderViewModel>();      foreach (AuthenticationDescription description in descriptions)     {         ExternalProviderViewModel model = new ExternalProviderViewModel         {             Url = Url.Action("ExternalLogin", new             {                 provider = description.AuthenticationType,                 response_type = "token",                 client_id = Startup.PublicClientId,                 redirect_uri = new Uri(Request.Url, returnUrl).AbsoluteUri + Startup.ExternalAuthPageUrl             })         };          providers.Add(model);     }     return View(providers); }  // Once a user picks a provider [AllowAnonymous] public ActionResult ExternalLogin(string provider, string error = null) {     return new ChallengeResult(loginProvider.Name, Url.Action("ExternalLoginCallback", "Account")); }  // On return from provider [AllowAnonymous] public async Task<ActionResult> ExternalLoginCallback(string returnUrl) {     var loginInfo = await _authManager.GetExternalLoginInfoAsync();     if (loginInfo == null)     {         return RedirectToAction("Login");     }      // Sign in the user with this external login provider if the user already has a login     var result = await _signInManager.ExternalSignInAsync(loginInfo, true);     switch (result)     {         case SignInStatus.Success:             return RedirectToLocal(returnUrl);         case SignInStatus.LockedOut:             return View("Lockout");         default:             // If the user does not have an account, then prompt the user to create an account             ViewBag.ReturnUrl = returnUrl;             ViewBag.LoginProvider = loginInfo.Login.LoginProvider;             return View("ExternalLoginConfirmation", new ExternalLoginConfirmationViewModel { Email = loginInfo.Email });     } }  // The External Login Confirmation view just confirms the user's email address, then submits to:  [HttpPost] [AllowAnonymous] [ValidateAntiForgeryToken] public async Task<ActionResult> ExternalLoginConfirmation(ExternalLoginConfirmationViewModel model, string returnUrl) {     if (ModelState.IsValid)     {         // Get the information about the user from the external login provider         var info = await _authManager.GetExternalLoginInfoAsync();         var user = new User { UserName = model.Email, Email = model.Email };         var result = await _userManager.CreateAsync(user);         if (result.Succeeded)         {             result = await _userManager.AddLoginAsync(user.Id, info.Login);             if (result.Succeeded)             {                 await _signInManager.SignInAsync(user, true, true);                 ExternalLoginModel externalLogin = ExternalLoginModel.FromIdentity( info);                 OwinHelper.SignIn(Request.GetOwinContext(), user, externalLogin, _userManager);                  return RedirectToLocal(returnUrl);             }         }     }     ViewBag.ReturnUrl = returnUrl;     return View(model); } 

OwinHelper.cs

public static class OwinHelper {     public static void SignIn(IOwinContext owinContext, User user, ExternalLoginModel externalLogin, ApplicationUserManager userManager)     {         owinContext.Authentication.SignOut(DefaultAuthenticationTypes.ExternalCookie);         var oAuthIdentity = CreateIdentity(user, OAuthDefaults.AuthenticationType);         var cookieIdentity = CreateIdentity(user, CookieAuthenticationDefaults.AuthenticationType);         SaveClaims(oAuthIdentity, user, userManager);         var properties = CreateProperties(user);         owinContext.Authentication.SignIn(properties, oAuthIdentity, cookieIdentity);     }      private static ClaimsIdentity CreateIdentity(User user, string authenticationType)     {         IList<Claim> claims = new List<Claim>();          claims.Add(new Claim(ClaimTypes.NameIdentifier, user.Id, null, ClaimsIdentity.DefaultIssuer, "Provider"));         claims.Add(new Claim(ClaimTypes.Name, user.FullName, null, ClaimsIdentity.DefaultIssuer, "Provider"));         claims.Add(new Claim(ClaimTypes.Email, user.Email, null, ClaimsIdentity.DefaultIssuer, "Provider"));         claims.Add(new Claim(ClaimTypes.GivenName, user.FullName, null, ClaimsIdentity.DefaultIssuer, "Provider"));          return new ClaimsIdentity(claims, authenticationType);     }      private static void SaveClaims(ClaimsIdentity identity, User user, ApplicationUserManager userManager)     {         foreach (var claim in identity.Claims)         {             if (identity.HasClaim(claim.Type, claim.Value))             {                 userManager.RemoveClaim(user.Id, claim);             }             userManager.AddClaim(user.Id, claim);         }     }      public static AuthenticationProperties CreateProperties(User user)     {         IDictionary<string, string> data = new Dictionary<string, string>         {             { "email", user.Email },             { "name", user.FirstName ?? string.Empty }         };         return new AuthenticationProperties(data);     } } 

I think that's everything that's relevant. Like I said, I'm looking for a review of my Identity and OAuth implementation, not general code quality. I've removed a lot of Null checks and alike to try and shorten the code a bit.

Please let me know if there's something you feel I might have missed, and any comments on my code I'm happy to receive.

              

Lista de respuestas

4
 
vote

Una clase estática llamada OwinHelper , con métodos CreateProperty1 , CreateProperty2 > CreateProperty3 y SaveClaims , huele gracioso. "Helper" es un mal signo, todo por sí mismo ("Manager" también es un signo), 9988776655544335 es otra mala señal, y los miembros públicos son una mala señal: la clase tiene todas las características de un Escala con demasiadas responsabilidades, que escalará por Cabello y tentáculos en crecimiento .

Estos métodos estáticos "Ayudos" probablemente pertenezcan como miembros privados / detalles de implementación de los tipos que "ayudan".

  public static AuthenticationProperties CreateProperties(User user) {     IDictionary<string, string> data = new Dictionary<string, string>     {         { "email", user.Email },         { "name", user.FirstName ?? string.Empty }     };     return new AuthenticationProperties(data); }   

Considere a estos miembros como parte del User Clase:

  private readonly Lazy<AuthenticationProperties> _authProperties; public AuthenticationProperties AuthProperties { get { return _authProperties.Value; } }   

Supongo que los miembros que eliminó de su POST son private readonly9 y solo asignado desde el constructor: Si el tipo 998877766555443310 no es inmutable, entonces tiene otra problema. De todos modos, la idea es crear que SignIn1 en el constructor Cuando está recibiendo el 99887776655443312 y SignIn3 , y porque su usuario es Enmutable No necesita preocuparse por si estos valores van a cambiar en el momento en que se necesitan el 99887776655443314 , para que pueda almacenar en caché una instancia.


Este método:

  SignIn5  

podría simplificarse a esto:

  SignIn6  

Esa es una sintaxis del inicializador de la colección que llama SignIn7 debajo de la campana para usted ... pero usted lo sabe, ya que lo está utilizando para inicializar un SignIn8 Algunas líneas aún más ...

AVISO I UTILIZO SignIn9 AQUÍ, PORQUE Yo lo uso en todas partes, 99887766555443320 es increíble! Es obvio lo que es el tipo.

Me parece irónico que no especifique CreateIdentity1 aquí:

  CreateIdentity2  

o aquí:

  CreateIdentity3  

Pero que lo harías aquí:

  CreateIdentity4  

o incluso aquí:

  CreateIdentity5  

No me malinterpretes: I amor 99887766555443326 y realmente ab Úsalo mucho. Puedo vivir con una base de código que no lo usa en todas partes , pero en una base de código que lo usa con moderación, me gusta ver la consistencia, y espero verlo donde sea útil (es decir, cuando el tipo es el desorden redundante de la placa de la caldera), ¡no donde el tipo no es obvio de la propia declaración!

 

A static class named OwinHelper, with methods SignIn, CreateIdentity and CreateProperty, and SaveClaims, smells funny. "Helper" is a bad sign all by itself ("Manager" is also a sign), static is another bad sign, and the public members are a bad sign: the class has all the characteristics of a type with already too many responsibilities, that will scale by growing hair and tentacles.

These static "helper" methods probably belong as private members / implementation details of the types they're "helping".

public static AuthenticationProperties CreateProperties(User user) {     IDictionary<string, string> data = new Dictionary<string, string>     {         { "email", user.Email },         { "name", user.FirstName ?? string.Empty }     };     return new AuthenticationProperties(data); } 

Consider these members as part of the User class:

private readonly Lazy<AuthenticationProperties> _authProperties; public AuthenticationProperties AuthProperties { get { return _authProperties.Value; } } 

I'm assuming the members you stripped from your post are private readonly and only ever assigned from the constructor: if the User type isn't immutable then you have another problem. Anyway the idea is to create that Lazy<T> in the constructor when you're receiving the Email and FirstName, and because your user is immutable you don't need to worry about whether these values are going to change by the time the AuthProperties are needed, so you can just cache an instance.


This method:

private static ClaimsIdentity CreateIdentity(User user, string authenticationType) {     IList<Claim> claims = new List<Claim>();      claims.Add(new Claim(ClaimTypes.NameIdentifier, user.Id, null, ClaimsIdentity.DefaultIssuer, "Provider"));     claims.Add(new Claim(ClaimTypes.Name, user.FullName, null, ClaimsIdentity.DefaultIssuer, "Provider"));     claims.Add(new Claim(ClaimTypes.Email, user.Email, null, ClaimsIdentity.DefaultIssuer, "Provider"));     claims.Add(new Claim(ClaimTypes.GivenName, user.FullName, null, ClaimsIdentity.DefaultIssuer, "Provider"));      return new ClaimsIdentity(claims, authenticationType); } 

Could be simplified to this:

private static ClaimsIdentity CreateIdentity(User user, string authenticationType) {     var claims = new List<Claim>     {         new Claim(ClaimTypes.NameIdentifier, user.Id, null, ClaimsIdentity.DefaultIssuer, "Provider"),         new Claim(ClaimTypes.Name, user.FullName, null, ClaimsIdentity.DefaultIssuer, "Provider"),         new Claim(ClaimTypes.Email, user.Email, null, ClaimsIdentity.DefaultIssuer, "Provider"),         new Claim(ClaimTypes.GivenName, user.FullName, null, ClaimsIdentity.DefaultIssuer, "Provider")     };      return new ClaimsIdentity(claims, authenticationType); } 

That's a collection initializer syntax calling .Add under the hood for you... but you know that, since you're using it to initialize a Dictionary a few lines further...

Notice I used var here, because I use it everywhere, var is awesome! it's obvious what the type is.

I find it ironic that you wouldn't specify var here:

IDictionary<string, string> data = new Dictionary<string, string> 

Or here:

IList<Claim> claims = new List<Claim>(); 

But that you would do it here:

var oAuthIdentity = CreateIdentity(user, OAuthDefaults.AuthenticationType); var cookieIdentity = CreateIdentity(user, CookieAuthenticationDefaults.AuthenticationType); var properties = CreateProperties(user); 

Or even here:

foreach (var claim in identity.Claims) 

Don't get me wrong: I love var and I really abuse it a lot. I can live with a codebase that doesn't use it everywhere, but in a codebase that uses it sparingly, I like to see consistency, and I expect to see it where it's useful (i.e. when the type is redundant boilerplate clutter) - not where the type isn't obvious from the statement itself!

 
 

Relacionados problema

7  Identity OAuth Implementación con Owin  ( Identity oauth implementation with owin ) 
Estoy construyendo una aplicación MVC 5 usando el marco de entidad de código 6 y un modelo de identidad ASP.NET personalizado para la autenticación. Solo es...

6  Autentique ASP.NET WEB API 2 usando Owin y Firsebase  ( Authenticate asp net web api 2 using owin and firebase ) 
Estoy a punto de ir a vivir con una aplicación, y, por lo tanto, apreciaría una revisión de mi configuración de autenticación de Firebase. No he podido encont...

2  Asegurar la API web con autenticación de token y permitir que la aplicación externa publique o obtenga datos  ( Securing web api using token authentication and allowing external application to ) 
Implementación de outauthentication con Owin e Identity para asegurar la API web y deje que la aplicación externa sea como la extensión de Chrome para acceder...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos