alphagov / notifications-net-client

.NET client for the GOV.UK Notify API
https://www.nuget.org/packages/GovukNotify/
MIT License
25 stars 20 forks source link

Release a new version compiled against the latest JWT #170

Open RocktimeLtd opened 1 year ago

RocktimeLtd commented 1 year ago

Hi Please can you release a new version that has been compiled against the latest version of JWT (currently 10.0.2)

Regards Kieron

samuelhwilliams commented 1 year ago

Hi Kieron,

Would you be able to put up a pull request that implements this change?

We'd be happy to review it and release it but as a team we no longer have anyone set up with .NET so there's a bit of friction on our side. If it's something you use more often, you may be in a better place to make the correct changes. We can then look at getting them validated and released.

If not, no worries, we can look at doing this, but it may take a bit longer.

Cheers.

RocktimeLtd commented 1 year ago

Hi Samuel,

Will do.

Best Regards,

Kieron Matthews Technical Director [A picture containing clipart Description automatically generated]

Rocktime Software Development Rocktime Ltd – Head Office, Sales & Marketing Arena Business Centres, Holyrood Close, Poole, Dorset BH17 7FJ Rocktime Ltd – Support & Development Regus Oxford Point, 19 Oxford Road, Bournemouth, Dorset BH8 8GS T: +44 (0) 1202 678777 W: www.rocktime.co.ukhttp://www.rocktime.co.uk/ W: www.versosoftware.comhttp://www.versosoftware.com/

@.http://www.versosoftware.co.uk/ [Logo Description automatically generated with medium confidence] @. http://www.digitalimpactawards.com/ @.*** http://www.digitalimpactawards.com/

Company No: 03781061 / VAT No: GB 750 1709 52

This communication may contain information which is privileged and confidential. If you are not the intended recipient or recipient's agent please contact the sender then immediately destroy the original message and any copies of it. Any opinions expressed in this email are those of the individual and not necessarily those of Rocktime Ltd. Rocktime Ltd accepts no liability for any loss or damage caused by software virus attachments.

From: Samuel Williams @.> Sent: Thursday, June 1, 2023 11:34 AM To: alphagov/notifications-net-client @.> Cc: rtDevTeam @.>; Author @.> Subject: Re: [alphagov/notifications-net-client] Release a new version compiled against the latest JWT (Issue #170)

Hi Kieron,

Would you be able to put up a pull request that implements this change?

We'd be happy to review it and release it but as a team we no longer have anyone set up with .NET. If it's something you use more often, you may be in a better place to make the correct changes. We can then look at getting them validated and released.

Cheers.

— Reply to this email directly, view it on GitHubhttps://github.com/alphagov/notifications-net-client/issues/170#issuecomment-1571786216, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AMOHH3G5ILY5GGXYTU74MFDXJBVYDANCNFSM6AAAAAAYWWOHKE. You are receiving this because you authored the thread.Message ID: @.**@.>>

RocktimeLtd commented 1 year ago

Hi Samuel,

I have updated my branch to the latest packages. I have a question though….

In the Authenticator.CreateToken(string secret, string serviceId) and Authenticator.DecodeToken(string token, string secret) Methods, you are currently using the HMACSHA256Algorithm is WEAK and Marked for deletion.

Severity Code Description Project File Line Suppression State Warning CS0618 'HMACSHA256Algorithm' is obsolete: 'HMAC SHA based algorithms are not secure to protect modern web applications. Consider switching to RSASSA or ECDSA.' GovukNotify (net462), GovukNotify (net6), GovukNotify (netstandard2.0) C:\LocalDev\GovUk\Notify\1\src\GovukNotify\Authentication\Authenticator.cs 23 Active

SHOULD I………

  1. Change this to a new STRONG (RSASSA or ECDSA) one, if I ensure that the change is made to both the CreateToken and DecodeToken?
  2. I need to use a specific Algorithm? If so , which should I use?
  3. Leave as is for now!

I have run the unit tests but the Integration tests fail due to missing environment variables Do you have some test ones I can use? private readonly String NOTIFY_API_URL = Environment.GetEnvironmentVariable("NOTIFY_API_URL"); private readonly String API_KEY = Environment.GetEnvironmentVariable("API_KEY"); private readonly String API_SENDING_KEY = Environment.GetEnvironmentVariable("API_SENDING_KEY");

        private readonly String FUNCTIONAL_TEST_NUMBER = Environment.GetEnvironmentVariable("FUNCTIONAL_TEST_NUMBER");
        private readonly String FUNCTIONAL_TEST_EMAIL = Environment.GetEnvironmentVariable("FUNCTIONAL_TEST_EMAIL");

        private readonly String EMAIL_TEMPLATE_ID = Environment.GetEnvironmentVariable("EMAIL_TEMPLATE_ID");
        private readonly String SMS_TEMPLATE_ID = Environment.GetEnvironmentVariable("SMS_TEMPLATE_ID");
        private readonly String LETTER_TEMPLATE_ID = Environment.GetEnvironmentVariable("LETTER_TEMPLATE_ID");
        private readonly String EMAIL_REPLY_TO_ID = Environment.GetEnvironmentVariable("EMAIL_REPLY_TO_ID");
        private readonly String SMS_SENDER_ID = Environment.GetEnvironmentVariable("SMS_SENDER_ID");
        private readonly String INBOUND_SMS_QUERY_KEY = Environment.GetEnvironmentVariable("INBOUND_SMS_QUERY_KEY");

namespace Notify.Authentication { public class Authenticator { public static string CreateToken(string secret, string serviceId) { ValidateGuids(new [] { secret, serviceId });

        var payload = new Dictionary<string, object>
        {
            { "iss", serviceId },
            { "iat", GetCurrentTimeAsSeconds() }
        };

        IJwtAlgorithm algorithm = new HMACSHA256Algorithm();
        IJsonSerializer serializer = new JsonNetSerializer();
        IBase64UrlEncoder urlEncoder = new JwtBase64UrlEncoder();
        IJwtEncoder encoder = new JwtEncoder(algorithm, serializer, urlEncoder);

        var notifyToken = encoder.Encode(payload, secret);

        return notifyToken;
    }

    public static double GetCurrentTimeAsSeconds()
    {
        var unixEpoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc);
        return Math.Round((DateTime.UtcNow - unixEpoch).TotalSeconds);
    }

    public static IDictionary<string, object> DecodeToken(string token, string secret)
    {
        try
        {
            IJsonSerializer serializer = new JsonNetSerializer();
            IDateTimeProvider provider = new UtcDateTimeProvider();
            IJwtValidator validator = new JwtValidator(serializer, provider);
            IBase64UrlEncoder urlEncoder = new JwtBase64UrlEncoder();
            IJwtAlgorithm algorithm = new HMACSHA256Algorithm();
            IJwtDecoder decoder = new JwtDecoder(serializer, validator, urlEncoder, algorithm);

            var jsonPayload = decoder.DecodeToObject<IDictionary<string, object>>(token, secret, verify: true);

            return jsonPayload;
        }
        catch (Exception e) when (e is SignatureVerificationException || e is ArgumentException)
        {
            throw new NotifyAuthException(e.Message);
        }
    }

    public static void ValidateGuids(String[] stringGuids)
    {
        if (stringGuids == null) return;

        foreach (var stringGuid in stringGuids)
        {
            if (!Guid.TryParse(stringGuid, out _))
                throw new NotifyAuthException("Invalid secret or serviceId. Please check that your API Key is correct");
        }
    }
}

}

Best Regards,

Kieron Matthews Technical Director [A picture containing clipart Description automatically generated]

Rocktime Software Development Rocktime Ltd – Head Office, Sales & Marketing Arena Business Centres, Holyrood Close, Poole, Dorset BH17 7FJ Rocktime Ltd – Support & Development Regus Oxford Point, 19 Oxford Road, Bournemouth, Dorset BH8 8GS T: +44 (0) 1202 678777 W: www.rocktime.co.ukhttp://www.rocktime.co.uk/ W: www.versosoftware.comhttp://www.versosoftware.com/

@.http://www.versosoftware.co.uk/ [Logo Description automatically generated with medium confidence] @. http://www.digitalimpactawards.com/ @.*** http://www.digitalimpactawards.com/

Company No: 03781061 / VAT No: GB 750 1709 52

This communication may contain information which is privileged and confidential. If you are not the intended recipient or recipient's agent please contact the sender then immediately destroy the original message and any copies of it. Any opinions expressed in this email are those of the individual and not necessarily those of Rocktime Ltd. Rocktime Ltd accepts no liability for any loss or damage caused by software virus attachments.

From: Samuel Williams @.> Sent: Thursday, June 1, 2023 11:34 AM To: alphagov/notifications-net-client @.> Cc: rtDevTeam @.>; Author @.> Subject: Re: [alphagov/notifications-net-client] Release a new version compiled against the latest JWT (Issue #170)

Hi Kieron,

Would you be able to put up a pull request that implements this change?

We'd be happy to review it and release it but as a team we no longer have anyone set up with .NET. If it's something you use more often, you may be in a better place to make the correct changes. We can then look at getting them validated and released.

Cheers.

— Reply to this email directly, view it on GitHubhttps://github.com/alphagov/notifications-net-client/issues/170#issuecomment-1571786216, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AMOHH3G5ILY5GGXYTU74MFDXJBVYDANCNFSM6AAAAAAYWWOHKE. You are receiving this because you authored the thread.Message ID: @.**@.>>

samuelhwilliams commented 1 year ago

Hi @RocktimeLtd - please leave this as-is for now. I think @leohemsted responded in more detail but as it stands, we are going to keep using HMAC SHA 256. Cheers.

RocktimeLtd commented 1 year ago

Hi Samuel,

I that case it is ready for you to review