alphagov / notifications-net-client

.NET client for the GOV.UK Notify API

GOV.UK Notify .NET client

Use this client to send emails, text messages and letters using the GOV.UK Notify API.

Useful links:

• Documentation
• Nuget package
• Changelog
• Contributing to this client

notifications-net-client's Issues

Provide asynchronous version of the client

I am working on a project for DfT that is using the Notify service. I would like an asynchronous version of the Notify client. The current implementation blocks when calling the Notify service.

I have taken a copy of the source code provided and modified it to make it asynchronous. Would it be useful to submit it as a Pull Request?

Nuget installation fails

Following the instructions to install with Nuget, I get a 404:

nuget install Notify -Source https://api.bintray.com/nuget/gov-uk-notify/notifications-net-client
Feeds used:
  https://api.bintray.com/nuget/gov-uk-notify/notifications-net-client

Installing package 'Notify' to 'C:\Users\fabio'.
  GET https://api.bintray.com/nuget/gov-uk-notify/notifications-net-client/FindPackagesById()?id='Notify'&semVerLevel=2.0.0
  NotFound https://api.bintray.com/nuget/gov-uk-notify/notifications-net-client/FindPackagesById()?id='Notify'&semVerLevel=2.0.0 124ms
An error occurred while retrieving package metadata for 'Notify' from source 'https://api.bintray.com/nuget/gov-uk-notify/notifications-net-client'.
  Failed to fetch results from V2 feed at 'https://api.bintray.com/nuget/gov-uk-notify/notifications-net-client/FindPackagesById()?id='Notify'&semVerLevel=2.0.0' with following message : Response status code does not indicate success: 404 (Not Found).
  Response status code does not indicate success: 404 (Not Found).

I also tried nuget install Notify -Source https://api.bintray.com/gov-uk-notify/nuget/Notify guessing based on the structure of the package URL and I got prompted for a username and password.

Release a new version compiled against the latest JWT

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

Regards Kieron

Notify Client Exception is not a standard json object

notifications-net-client/src/GovukNotify/Client/BaseClient.cs

Lines 118 to 129 in 358238c

	private void HandleHTTPErrors(HttpResponseMessage response, string errorResponseContent)
	{
	try
	{
	var errorResponse = JsonConvert.DeserializeObject<NotifyHTTPErrorResponse>(errorResponseContent);
	throw new NotifyClientException("Status code {0}. The following errors occured {1}", errorResponse.getStatusCode(), errorResponse.getErrorsAsJson());
	}
	catch (Exception ex)
	{
	throw new NotifyClientException("Status code {0}. Error: {1}, Exception: {2}", response.StatusCode.GetHashCode(), errorResponseContent, ex.Message);
	}
	}

When an exception occurs, a NotifyClientException is thrown, but its content is a plain string as outlined above. The string contains the information from the documentation, but in a very unintuitive way, making it difficult to parse. For example, here is the string from the 'message' field one one such case:

Status code 400. Error: {"errors":[{"error":"ValidationError","message":"email_address Not a valid email address"}],"status_code":400}, Exception: Status code 400. The following errors occured [{"error": "ValidationError", "message": "email_address Not a valid email address"}]

I would much rather work with just the following, since it is much easier to parse, or maybe even incorporate those as fields in the NotifyClientException class.:
{"errors":[{"error":"ValidationError","message":"email_address Not a valid email address"}],"status_code":400}

HMACSHA256Algorithm is obsolete

HMACSHA256Algorithm is obsolete: 'HMAC SHA based algorithms are not secure to protect modern web applications. Consider switching to RSASSA or ECDSA:

    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;
    }

Nuget package update

I recently contributed to the project (#57) and was wondering if there was an ETA for 2.3.0 being published to the bintray so we can consume it?

Publish to nuget?

Could this be published as a nuget package? would make it a lot easier to consume

Apologies I did miss the BinTray reference, this is currently blocked for us so will have to get it whitelisted before we can use it.

Migrate to .NET core

As part of the DSP/ACP containerised hosting for the UKHO there is a need to build .NET solutions using .NET core and not .NET standard.

Could this library be ported to .NET core?

Task result obtained synchronously inside an asynchronous method

At this line

notifications-net-client/src/Notify/Client/BaseClient.cs

Line 74 in 6931da0

var response = SendRequest(url, method, content).Result;

inside an asynchronous method a result of SendRequest is taken synchronously.

public async Task<string> MakeRequest(string url, HttpMethod method, HttpContent content = null)
{
  var response = SendRequest(url, method, content).Result;

  var responseContent = await response.Content.ReadAsStringAsync().ConfigureAwait(false);

  if (!response.IsSuccessStatusCode)
  {
    HandleHTTPErrors(response, responseContent);
  }
  return responseContent;
}

I'm wondering if there was a specific reason for that, or can it become just var response = await SendRequest(url, method, content)?

There's a very minor issue with that - if SendRequest throws an exception (e.g. TaskCancelled upon timeout), then caller would get an AggregateException, which is a bit counter-intuitive and less convenient to process.

Also, such a pattern seem a bit dangerous as theoretically it could lead to deadlocks depending on circumstances. In our application we haven't seen any and there are ConfigureAwait-s, so that mightn't be a real issue.

Remove "Add the Bintray package source (optional)" from tech docs

Removed the "Add the Bintray package source (optional)" section from the tech docs after discussion with @leohemsted and @karlbaker02:

"If you are referencing the GOV.UK Notify client package from a continuous integration (CI) tool, you may need to add the Bintray package source to your Nuget configuration.

To do this, add a nuget.config file [external link] to the same folder as your .sln file. This is the project root directory by default. The nuget.config file must contain the following content:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <config>
    <add key="repositoryPath" value="$\..\packages" />
  </config>
  <packageSources>
    <add key="nuget.org" value="https://api.nuget.org/v3/index.json" protocolVersion="3" />
    <add key="bintray" value="https://api.bintray.com/nuget/gov-uk-notify/nuget" />
  </packageSources>
</configuration>
```"

Does this need to be added to the contributing.md file?

Missing XML Comments on .NET GOV-UK Notify

As per my comments in the Slack channel for GOV Notify, it would be extremely useful if we could have XML comments attached to the libraries for .NET versions. This helps us as developers, as the code is practically self-documented, then, in editors such as Visual Studio or VS Code.

This can be achieved by typing '///' at the start of a code block (e.g. before a property, method, field or class).

Many thanks.

Newsletter for API breaking changes

Not really an issue I'm having but I need to get our team to be notified about changes.

Are the Gov Notify team members notified of such changes?

Cancellation support for async methods

It would be very useful if the various *Async methods could optionally take a CancellationToken to cancel the send operation.