Sunday, 12 March 2017

Aggregating Reports in Multi-Module Maven Projects

With a highly complex multi-module Maven build, it can be very useful to use the Site generation feature of Maven to produce HTML reports for Unit and Integration Test results, as well as code coverage.  This can save scrolling back through pages of console output to try and figure out which Unit Test has unexpectedly failed

However, by default, Maven will produce a separate report for each module, which can mean having to click through many different modules to figure out which one contains the failure - especially when the module Name (shown in the Maven log) doesn't correspond directly to the path where it can be found.

Fortunately, Maven does have some report aggregating capabilities.  Less fortunately, these seem to be poorly documented online - either that, or everyone else figured it out with no help at all!

Module Configuration

I've set up a simple demo project to explore how this can be done effectively.  My scope included the following requirements:
  • Aggregate the Unit Test reports (Surefire) in a single place
  • Aggregate the Integration Test reports (Failsafe) in a single place
  • Aggregate the Code Coverage report (Jacoco) in a single place
Additionally, to match the real-world scenario that prompted this investigation, I wanted to cover the interesting edge case of an "Integration Test Only" module - ideally, failures in this module would be aggregated in with the Failsafe reports, and the aggregated code coverage report from Jacoco would include the coverage from the integration tests.

So, my starting position looks like this:
  • module1
  • module2
  • component-test (depends on module1 & module2)
The next step is to combine these three modules into a single Maven build, and here's where it gets interesting.  It seems that there are two separate principles in play in Maven, which are often conflated into a single implementation: Inheritance vs. Aggregation

The best article I've found on the topic is a StackOverflow answer, which led me to the two possible solutions below:
  • parentAggregate
    • module1 (inherits from parentAggregate)
    • module2 (inherits from parentAggregate)
    • component-test (inherits from parentAggregate, depends on module1 & module2)
or
  • aggregate
    • parent
    • module1 (inherits from ../parent)
    • module2 (inherits from ../parent)
    • component-tests (inherits from ../parent, depends on module1 & module2)
The key difference between these two solutions is the build order.  The parent module is always built before any child modules - so parentAggregate is built before module1 or module2.  However, with the separate parent and aggregate model, the aggregate module is built last, after all modules are completed.  This is quite interesting and important when it comes to creating aggregate reports for Jacoco.

The project I've inherited uses the parentAggregate model, which at first glance seems to be the more elegant solution.  There's no need to specify the relative path for the parent, and the directory structure directly matches the inheritance model.  However, as we'll see, this isn't necessarily the optimal solution when it comes to aggregate reports.

For the rest of this post, I'm going to try and cover both models - highlighting the similarities and differences between the configuration required in each case.  At the end, I'll offer my opinion as to which solution is superior for my requirements, and why.

Executing Tests and Gathering Coverage

Let's start by configuring the Surefire, Failsafe and Jacoco plugins to actually execute the Unit Tests, Integration Tests and Code Coverage during the build.

There are a few things to note here:
  • For the Surefire plugin, there's no need to specify any executions; the defaults work as expected
  • For both Failsafe and Jacoco, if you don't specify the executions, the plugin will not run
  • In the parentAggregate model, this should be added to the parentAggregate module, so it is inherited by each sub-module.  Similarly, in the separate aggregate/parent model, it must be added to the parent model, so it is inherited by each sub-module.
This configuration results in four outcomes during the Maven build cycle:
  • During the test phase, the Unit Tests are executed by the Surefire plugin.
  • While these are running, Jacoco gathers the code coverage and creates jacoco.exec
  • During the verify phase, the Integration Tests are executed by the Failsafe plugin
  • While these are running, Jacoco gathers the code coverage and created jacoco-it.exec
At this point though, we have no reports generated at all; if we want to examine the results of the unit tests, we need to look at each separate TXT or XML file generated by Surefire and Failsafe, and we have no way to read the coverage data.

Basic Reporting per Module

The next step is to configure Surefire Reports, Failsafe Reports and Jacoco Reports for each sub-module.  For now, we're just interested in making it easy to read the reports per sub-module, rather than aggregating the reports.


Again, the points of interest:
  • These are still added to the "parent" module, whichever that is.
  • For speed, I've disabled the default reports - only the index report is generated, which provides the index.html page with the project description.
  • The Failsafe reports are generated by the Surefire report plugin
  • For all three reports, the report is only generated if the relevant raw data exists.  This means that there will be no Jacoco Integration report if there is no jacobo-it.exec file for a module.  
  • Also, since my component-test module has no production classes (it's test only), there will be no Jacoco report for this module either, as there's no way to calculate a percentage coverage without having the original classes to compare to.  Modules that do have both Unit and Integration tests would get both Jacoco reports
At this point, we can easily get separate reports for each module, but we have no visibility of what is covered by our dedicated integration test module, and it's still tricky to find a test failure in a random module.

Executing Maven

A note now on how best to execute Maven.  If you do the obvious mvn clean install site, you will discover that failed unit tests prevent the Site from being generated for that module - so there's no reports to examine!  However, mvn site on it's own will quite happily find the failed tests and add them to the report, but runs the tests a second time anyway.  I've found the best combination is this:
mvn -fn clean verify; mvn -DskipTests site

This ensures all the tests are run (-fn, Fail Never, means that the build will continue on failing Unit Tests, so you can see all the tests that fail, not just the first one), then runs the Site build as a separate task, without re-running the tests.
If your integration tests are particularly slow, you may want to run just test instead of verify.
You may want to add a third step - mvn site:stage - to put the Sites for all the modules into a single location. However, when we get to the point of generating aggregate reports, this becomes unnecessary.

Aggregated Test Reports

Finally, we get to the most interesting part of this post - generating the Aggregate reports.

For Surefire and Failsafe, this is quite easy to achieve.

This needs to be added to the aggregate module - for both the parentAggregate and the separate models, that's the top level module.  When using the separate parent and aggregate, the inherited=false setting is technically not required, but won't do any harm.
This will generate the Aggregate reports when the aggregate module site is built.  When using the separate parent and aggregate, this is the last module to be built, so it could be done as part of a single Maven execution - but as discussed above, it's useful to run a separate build for the Site to catch any Unit Test failures in the reports.
When using the combined parentAggregate model, the parent is built first, so if you try and do this all as one execution, the Aggregate report will be empty, which is one reason some people would prefer this architecture.
In this model, since both the reportSets are set not to inherit, the child modules end up using the default reportSet only, and therefore generating the non-aggregated reports, as we require.

Aggregated Coverage Report

Generating an Aggregate report from Jacoco is, unfortunately, far more involved.  Jacoco provides a target for an aggregate report (report-aggregate), but this target only aggregates the reports for dependencies of the current module.

This gives us two different solutions - one for the parentAggregate model, and one for the separate model.

When using a parentAggregate, the recommended solution from Jacoco themselves is to create a separate reports module.  This module must depend on all the other modules - and, significantly, specify the test scope for Test-Only modules.  This ensures that Jacoco knows how to count lines of code - in Test-Only modules, the execution counts toward coverage, but the code itself does not need to be covered by tests.  The Jacoco Aggregate report is added in the dedicated reports module.

When using separate parent and aggregator modules, the aggregator is built last, so one would think that adding the Jacoco Aggregate report to this module would be sufficient, but alas, Jacoco still needs the dependancies to know the scope of each module.

Therefore, to generate the Aggregate report for Jacoco we must add the following to either the aggregator or the reports module, depending on the model chosen.

In either architecture, the Dependancies of the aggregator or the reports module must be maintained separately for the Aggregated Code Coverage report to continue to be accurate.  This is a burden on maintenance that would be better avoided.
In my opinion, using separate the aggregator module has the advantage here, as the Dependencies can be updated when the Modules are updated, whereas with a reports module, these would have to be updated separately.

Final Conclusions

By executing the Build and the Site generation separately, we eliminate a lot of the confusion around aggregate reports almost accidentally - which may be why the internet as a whole is so quiet on this topic... perhaps most people do these separately by default!

mvn -fn clean verify; mvn -DskipTests site

Generating the Aggregated Test Reports is simple, whichever model you choose to implement.

Generating the Aggregated Jacoco Reports is difficult, whichever model you implement, although the combined parentAggregator does make this slightly worse.

In the end, the best solution is probably to generate the Aggregated Test Reports in Maven, and leave the Code Coverage report to a dedicated tool such as Sonar!


Monday, 28 November 2016

Azure Federated Security Signing Key Rollover

Lately, I've been working with Azure's implementation of OAuth and OpenID, using mod_auth_mellon in particular.

Azure has been updating their signing keys rather frequently (twice in the last month), and each time the keys have been updated, our authentication solution has failed until we've refreshed the Federation Metadata file from Azure.

The documentation from Azure indicates that applications really ought to be able to handle key rollover automatically (see https://docs.microsoft.com/nb-no/azure/active-directory/active-directory-signing-key-rollover) - and indeed, Mellon does handle this gracefully, but only when the key is already known in the Federation Metadata file (known as the MellonIdPMetadataFile in Mellon).  However, the failures occur when Azure moves to a new key that Mellon didn't know about in advance.

What's actually happening is that Azure has been adding new keys, and starting to use them after 1-2 weeks.  However, since we don't refresh the Metadata automatically, the first Mellon knows about the new key is when it gets used... at which point, we fail to verify the signature, and authentication fails.

The simple & naive solution would be to refresh the Federation Metadata automatically - but that left me with an uncomfortable feeling, which I shall explain below.
However, after a full analysis, it turns out that this naive solution is indeed correct (with a few caveats, which I shall also explain).


Why was I nervous?  Consider the authentication flow:
  • We send a request to Azure to authenticate a user 
  • We get a response back with a token, and verify that it really is from Azure by checking the signature on that Token 
  • We then grant access to the user
Having a signature on the token guards against any kind of Man-in-the-Middle attack - If a Man-in-the-Middle was compromised (e.g. DNS spoofing, proxy redirection etc.), we would currently be safe:
  • We send a request to Azure to authenticate the user
    • It gets redirected to Attacker’s server instead
  • We get a response back which actually comes from the Attacker’s server
    • We reject that response, because it is not (cannot be!) signed with a valid key
However, if we are automatically refreshing the Federation Metadata document, we no longer have any safeguard that the token really comes from Azure
  • We fetch the latest federation metadata file from the Azure server
    • Again, we get redirected to the Attacker’s server instead, and get a metadata document with his key added
  • We send a request to Azure to authenticate the user 
    • It gets redirected to Attacker’s server instead
  • We get a response back which actually comes from the Attacker’s server
    • We validate the response, because it’s signed with a key that we’ve been told is valid
A patient MitM could even keep the authentication requests going to the real authentication server for days/weeks until he detects a refresh of the metadata file and therefore knows his key will now be accepted…

Based on this attack scenario, it seems that automatic refreshes of the metadata file need to either be authenticate, or be signed in some way so that we know that this metadata file does really come from a trusted source.

Of course, we already have a mechanism to do this.  The Federation Metadata from Azure is served over HTTPS, which means not only is the data encrypted in transit, but (more relevantly) the connection to the server can be authenticated as "yes, this really is the Microsoft Azure server"

The code sample in the Azure documentation has the slightly cryptic comment here:
MetadataSerializer serializer = new MetadataSerializer()
{
    // Do not disable for production code
    CertificateValidationMode = X509CertificateValidationMode.None
}; 
 
Here,  the example code is disabling the check that the Metadata is really coming from the expected server... which is rather a bad idea in any code, not just Production.

Indeed, fetching the Federation Metadata automatically is safe, but only if it is done over HTTPS, and the certificate from the HTTPS server is verified as coming from a trusted certificate authority.

Indeed, any MitM can serve a doctored Metadata document over HTTPS with a certificate indicating that it's coming from login.microsoftonline.com - but only Azure will be able to provide a certificate that has been issued to login.microsoftonline.com and has been signed by a suitably trusted Root CA.

To be fully secure then, we should use curl with the "--with-ca-bundle" parameter or wget with the "--ca-certificate" parameter to be 100% sure that the Metadata document has come from a trusted server.

Wednesday, 13 July 2016

TravelEx Supercard - Not so Super anymore!

About a year ago, I was fortunate enough to be able to join in the pilot of the TravelEx SuperCard

This card offers 0% fees on foreign currency transactions, which saves a few pounds of the typical UK credit cards which charge anything from 50p to £2.50, plus up to 3% of the transaction amount.

Of course, I already had a 0% fee card for foreign currency, so this wasn't new... but what is new and special is that with Supercard, your transactions are recharged back to some other card in sterling - allowing you to gain cashback or loyalty points with that card as normal.
Additionally, all your transactions appear immediately in an App, showing you exactly how much you would be charged in pounds (and of course, emphasising the savings you'd made with Supercard).

This immediate display of the sterling amount turned out to be the most useful feature of the card, for me, since it allowed me to claim my work expenses immediately (instead of waiting for the transaction to clear).  It was also great on holiday, since I could keep a running total of the real cost of the trip.

Supercard has now launched publicly, with one significant change - they are now a Mastercard instead of Visa.

I haven't yet had chance to use this card abroad myself, but reports are coming in that people are being charged a mysterious extra charge.
Thankfully, the community at Head for Points has been quick to analyse these charges - my thanks to John in the comments there, who seems to have done a lot of research and analysis on this!

My interpretation of this is that Supercard is receiving the exchange rate data from Mastercard one day late.  This means that the transaction is initially being charged at "yesterday's" rate - and this is what is shown in the app as well.
Later, when Supercard receives the correct rate for the transaction date, they are doing an adjustment - so in fact, they are sticking perfectly to their own terms and conditions.
They've also stated on Twitter that if the rate had changed the other way, a refund would be issued instead (although, with Brexit, we've yet to see that in action!)
https://twitter.com/SupercardUK/status/752956412947472384
https://twitter.com/SupercardUK/status/752956480190545921

 

Assuming that this is all correct and true, financially we're probably still better off with the Mastercard solution - the statistics seem to show that the Mastercard rates are very slightly better than the Visa rates.

However, the fact that Supercard is now charging a second transaction to issue the correct exchange rate means that the app is no longer as useful for me as a traveller.  I can no longer rely on the figures shown to be the final charge, as I could before, since it is likely that a correction will be made a few days later.

This appears to be a limitation from Mastercard, since their exchange rate is defined based on the "Settlement Rate", which appears to be determined (or at least published) one day in arrears.

However, since at the moment Supercard is not updating the transaction on the App when the final exchange rate is determined, they appear to be in breach of their own terms and conditions!
In particular, section 7.5 indicates:
... Your statement will show:
...
(d) the exchange rate (where applicable) which applied to each Transaction;
(e) where applicable, the amount of each Transaction following a currency conversion (in GBP sterling) 

My only hope is that Supercard will update the App to reflect this more clearly (since it's unlikely that they can get "today's" Settlement Rate from Mastercard).
I would suggest the following improvements:
  • Mark the initial transaction as an "indicative rate" - perhaps faded grey, as is often used for pending transactions on online statements
  • As soon as the correct rate is available, show that as the final rate in the app - ideally, it should be possible to do this within 24 hours of the transaction taking place
  • Instead of charging the linked credit card the full transaction amount immediately, put a hold on the "indicative" amount, and put through the full charge only once the real exchange rate is known.
Sadly, I think the chances of this being changed correctly are very, very slim!