search

samdjstevens / java-totp

A java library for implementing Time-based One Time Passwords for Multi-Factor Authentication.
MIT License
422 stars 103 forks source link

Fix #24 - Limit the extensibility of classes and methods #26

Open OlivierJaquemet opened 4 years ago

OlivierJaquemet commented 4 years ago

Apply Guideline 4-5 / EXTEND-5: Limit the extensibility of classes and methods from the Secure Coding Guidelines for Java SE https://www.oracle.com/technetwork/java/seccodeguide-139067.html#4-5

(reported by running VisualCodeGrepper https://github.com/nccgroup/VCG/blob/master/VisualCodeGrepper/modJavaCheck.vb#L318 )

samdjstevens commented 4 years ago

Thanks for the PR - I'm not 100% this is needed/beneficial. Whilst it does improve security and composition should be favoured over inheritence generally, is it being too restrictive by not allowing consumers to override RecoveryCodeGenerator for example?

OlivierJaquemet commented 4 years ago

@samdjstevens honestly, I'm not sure either I should have made all classes final. As stated in the Secure Coding Guidelines, we have two choices "Design classes and methods for inheritance or declare them final"... So I have absolutely no problem in reverting some of them !!

I may have been a little overzealous :p

OlivierJaquemet commented 4 years ago

PR updated to revert use of final keyword for all classes which could be subclassed.

samdjstevens commented 4 years ago

Appreciate the work on this, but because these changes would require a new major version (non backwards compatible API changes) I think I'm going to hold off merging until other breaking changes are introduced into the library. Thanks!