From 93dd16f8296fae0d55c84534f9a6b050e5b54cc9 Mon Sep 17 00:00:00 2001 From: Haacked Date: Sun, 19 Apr 2015 19:42:13 -0700 Subject: [PATCH] Add more info to TwoFactorChallengeFailedException I was running into an issue where i wanted more information from the TwoFactorChallengeFailedException. It turns out, the exception could easily provide both the TwoFactorType AND the authentication code provided. So :boom:! --- Octokit/Clients/AuthorizationsClient.cs | 2 +- Octokit/Exceptions/ApiException.cs | 3 + .../TwoFactorAuthorizationException.cs | 115 ++++++++++++++++++ .../TwoFactorChallengeFailedException.cs | 31 +++-- .../Exceptions/TwoFactorRequiredException.cs | 48 +------- Octokit/Http/Connection.cs | 4 +- Octokit/Octokit-Mono.csproj | 1 + Octokit/Octokit-MonoAndroid.csproj | 1 + Octokit/Octokit-Monotouch.csproj | 1 + Octokit/Octokit-Portable.csproj | 1 + Octokit/Octokit-netcore45.csproj | 1 + Octokit/Octokit.csproj | 1 + 12 files changed, 155 insertions(+), 54 deletions(-) create mode 100644 Octokit/Exceptions/TwoFactorAuthorizationException.cs diff --git a/Octokit/Clients/AuthorizationsClient.cs b/Octokit/Clients/AuthorizationsClient.cs index 029cc313..9d4c342a 100644 --- a/Octokit/Clients/AuthorizationsClient.cs +++ b/Octokit/Clients/AuthorizationsClient.cs @@ -172,7 +172,7 @@ namespace Octokit } catch (AuthorizationException e) { - throw new TwoFactorChallengeFailedException(e); + throw new TwoFactorChallengeFailedException(twoFactorAuthenticationCode, e); } } diff --git a/Octokit/Exceptions/ApiException.cs b/Octokit/Exceptions/ApiException.cs index 9e16c540..fa30f9f2 100644 --- a/Octokit/Exceptions/ApiException.cs +++ b/Octokit/Exceptions/ApiException.cs @@ -67,6 +67,7 @@ namespace Octokit StatusCode = response.StatusCode; ApiError = GetApiErrorFromExceptionMessage(response); + HttpResponse = response; } /// @@ -97,6 +98,8 @@ namespace Octokit StatusCode = statusCode; } + public IResponse HttpResponse { get; private set; } + public override string Message { get { return ApiErrorMessageSafe ?? "An error occurred with this API request"; } diff --git a/Octokit/Exceptions/TwoFactorAuthorizationException.cs b/Octokit/Exceptions/TwoFactorAuthorizationException.cs new file mode 100644 index 00000000..beae116c --- /dev/null +++ b/Octokit/Exceptions/TwoFactorAuthorizationException.cs @@ -0,0 +1,115 @@ +using System; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Net; +using System.Runtime.Serialization; + +namespace Octokit +{ +#if !NETFX_CORE + /// + /// Represents a failed 2FA challenge from the API + /// + [Serializable] +#endif + [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", + Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] + public abstract class TwoFactorAuthorizationException : AuthorizationException + { + /// + /// Constructs an instance of TwoFactorRequiredException. + /// + /// Expected 2FA response type + /// The inner exception + protected TwoFactorAuthorizationException(TwoFactorType twoFactorType, Exception innerException) + : base(HttpStatusCode.Unauthorized, innerException) + { + TwoFactorType = twoFactorType; + } + + /// + /// Constructs an instance of TwoFactorRequiredException. + /// + /// The HTTP payload from the server + /// Expected 2FA response type + protected TwoFactorAuthorizationException(IResponse response, TwoFactorType twoFactorType) + : base(response) + { + Debug.Assert(response != null && response.StatusCode == HttpStatusCode.Unauthorized, + "TwoFactorRequiredException status code should be 401"); + + TwoFactorType = twoFactorType; + } + + /// + /// Constructs an instance of TwoFactorRequiredException. + /// + /// The HTTP payload from the server + /// Expected 2FA response type + /// The inner exception + protected TwoFactorAuthorizationException(IResponse response, TwoFactorType twoFactorType, Exception innerException) + : base(response, innerException) + { + Debug.Assert(response != null && response.StatusCode == HttpStatusCode.Unauthorized, + "TwoFactorRequiredException status code should be 401"); + + TwoFactorType = twoFactorType; + } + + /// + /// Expected 2FA response type + /// + public TwoFactorType TwoFactorType { get; private set; } + + #if !NETFX_CORE + /// + /// Constructs an instance of TwoFactorRequiredException. + /// + /// + /// The that holds the + /// serialized object data about the exception being thrown. + /// + /// + /// The that contains + /// contextual information about the source or destination. + /// + protected TwoFactorAuthorizationException(SerializationInfo info, StreamingContext context) + : base(info, context) + { + if (info == null) return; + TwoFactorType = (TwoFactorType) (info.GetInt32("TwoFactorType")); + } + + public override void GetObjectData(SerializationInfo info, StreamingContext context) + { + base.GetObjectData(info, context); + info.AddValue("TwoFactorType", TwoFactorType); + } +#endif + + } + + /// + /// Methods for receiving 2FA authentication codes + /// + public enum TwoFactorType + { + /// + /// No method configured + /// + None, + /// + /// Unknown method + /// + Unknown, + /// + /// Receive via SMS + /// + [SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Sms")] + Sms, + /// + /// Receive via application + /// + AuthenticatorApp + } +} diff --git a/Octokit/Exceptions/TwoFactorChallengeFailedException.cs b/Octokit/Exceptions/TwoFactorChallengeFailedException.cs index 7a93bbae..e068f206 100644 --- a/Octokit/Exceptions/TwoFactorChallengeFailedException.cs +++ b/Octokit/Exceptions/TwoFactorChallengeFailedException.cs @@ -1,6 +1,5 @@ using System; using System.Diagnostics.CodeAnalysis; -using System.Net; using System.Runtime.Serialization; namespace Octokit @@ -13,23 +12,26 @@ namespace Octokit #endif [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] - public class TwoFactorChallengeFailedException : AuthorizationException + public class TwoFactorChallengeFailedException : TwoFactorAuthorizationException { /// /// Constructs an instance of TwoFactorChallengeFailedException /// - public TwoFactorChallengeFailedException() : - base(HttpStatusCode.Unauthorized, null) + public TwoFactorChallengeFailedException() : base(TwoFactorType.None, null) { } /// /// Constructs an instance of TwoFactorChallengeFailedException /// + /// The authorization code that was incorrect /// The inner exception - public TwoFactorChallengeFailedException(Exception innerException) - : base(HttpStatusCode.Unauthorized, innerException) + public TwoFactorChallengeFailedException( + string authorizationCode, + ApiException innerException) + : base(ParseTwoFactorType(innerException), innerException) { + AuthorizationCode = authorizationCode; } public override string Message @@ -37,9 +39,16 @@ namespace Octokit get { return "The two-factor authentication code supplied is not correct"; } } + public string AuthorizationCode { get; private set; } + + static TwoFactorType ParseTwoFactorType(ApiException exception) + { + return exception == null ? TwoFactorType.None : Connection.ParseTwoFactorType(exception.HttpResponse); + } + #if !NETFX_CORE /// - /// Constructs an instance of TwoFactorChallengeFailedException + /// Constructs an instance of TwoFactorRequiredException. /// /// /// The that holds the @@ -52,6 +61,14 @@ namespace Octokit protected TwoFactorChallengeFailedException(SerializationInfo info, StreamingContext context) : base(info, context) { + if (info == null) return; + AuthorizationCode = info.GetString("AuthorizationCode"); + } + + public override void GetObjectData(SerializationInfo info, StreamingContext context) + { + base.GetObjectData(info, context); + info.AddValue("AuthorizationCode", AuthorizationCode); } #endif } diff --git a/Octokit/Exceptions/TwoFactorRequiredException.cs b/Octokit/Exceptions/TwoFactorRequiredException.cs index ad7a2ee3..f2361c24 100644 --- a/Octokit/Exceptions/TwoFactorRequiredException.cs +++ b/Octokit/Exceptions/TwoFactorRequiredException.cs @@ -14,7 +14,7 @@ namespace Octokit #endif [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] - public class TwoFactorRequiredException : AuthorizationException + public class TwoFactorRequiredException : TwoFactorAuthorizationException { /// /// Constructs an instance of TwoFactorRequiredException. @@ -27,10 +27,8 @@ namespace Octokit /// Constructs an instance of TwoFactorRequiredException. /// /// Expected 2FA response type - public TwoFactorRequiredException(TwoFactorType twoFactorType) - : base(HttpStatusCode.Unauthorized, null) + public TwoFactorRequiredException(TwoFactorType twoFactorType) : base(twoFactorType, null) { - TwoFactorType = twoFactorType; } /// @@ -38,12 +36,11 @@ namespace Octokit /// /// The HTTP payload from the server /// Expected 2FA response type - public TwoFactorRequiredException(IResponse response, TwoFactorType twoFactorType) : base(response) + public TwoFactorRequiredException(IResponse response, TwoFactorType twoFactorType) + : base(response, twoFactorType) { Debug.Assert(response != null && response.StatusCode == HttpStatusCode.Unauthorized, "TwoFactorRequiredException status code should be 401"); - - TwoFactorType = twoFactorType; } public override string Message @@ -66,44 +63,7 @@ namespace Octokit protected TwoFactorRequiredException(SerializationInfo info, StreamingContext context) : base(info, context) { - if (info == null) return; - TwoFactorType = (TwoFactorType) (info.GetInt32("TwoFactorType")); - } - - public override void GetObjectData(SerializationInfo info, StreamingContext context) - { - base.GetObjectData(info, context); - info.AddValue("TwoFactorType", TwoFactorType); } #endif - - /// - /// Expected 2FA response type - /// - public TwoFactorType TwoFactorType { get; private set; } - } - - /// - /// Methods for receiving 2FA authentication codes - /// - public enum TwoFactorType - { - /// - /// No method configured - /// - None, - /// - /// Unknown method - /// - Unknown, - /// - /// Receive via SMS - /// - [SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Sms")] - Sms, - /// - /// Receive via application - /// - AuthenticatorApp } } diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index 43a808c2..29dc6f1b 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -489,9 +489,9 @@ namespace Octokit : new ForbiddenException(response); } - static TwoFactorType ParseTwoFactorType(IResponse restResponse) + internal static TwoFactorType ParseTwoFactorType(IResponse restResponse) { - if (restResponse.Headers == null || !restResponse.Headers.Any()) return TwoFactorType.None; + if (restResponse == null || restResponse.Headers == null || !restResponse.Headers.Any()) return TwoFactorType.None; var otpHeader = restResponse.Headers.FirstOrDefault(header => header.Key.Equals("X-GitHub-OTP", StringComparison.OrdinalIgnoreCase)); if (String.IsNullOrEmpty(otpHeader.Value)) return TwoFactorType.None; diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index d9f81fc2..3f0dc203 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -383,6 +383,7 @@ + \ No newline at end of file diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index 897b2a73..2dbbd085 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -395,6 +395,7 @@ + \ No newline at end of file diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index 997f94e7..13f7b0cf 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -388,6 +388,7 @@ + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 55d10491..68a6e7dd 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -381,6 +381,7 @@ + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index 65291e0a..44206c7f 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -385,6 +385,7 @@ + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index fdf8c32f..55830cee 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -73,6 +73,7 @@ +