Replies: 35 comments 23 replies
-
Hi, @apr-un Good news, I will try to answer point by point.
4, / 5 / 6 Yes, Some internals must be trusted, to avoid abusive use of assemblies... And yes it's a public key !
8 / 9) The MFA azure part is based on the ADAL library (Vittorio Bertocci), I think you just have to make sure that the right package is present.
12/13) No, not from resources, they can be replaced. can be a placeholder somewhere in the source code 14/15) I just have to tell you, that you did the job. I reflect on your proposals, in order to be able to carry out easier custom builds Hey, Ground control to major Tom, You've really made the grade ! Changing the issue to discussion, this can help ! best regards redhook |
Beta Was this translation helpful? Give feedback.
-
Thank You for reply, I assume that my description of application building is mostly correct :)
I tried to find correct version, but project is referencing just global library Microsoft.IdentityServer.Aad.Sas.dll (not nuget package!) which is part of ADFS (I think - not too many sources write about it on net). I only found several mentions on web that AcquireToken method is deprecated in ADAL 3.x (loot of links, just FYI): MicrosoftDocs/azure-docs#24458 Current repo for ADAL mentions that it is being replaced by new MSAL library: I put small pull request with some fixes and updated readme ( #178 ) , and will follow up soon with enchancements to biometric user experience Regards |
Beta Was this translation helpful? Give feedback.
-
Hi @redhook62, Code was tested and is running on production system without problems (at least till now :D) This code was prepared with some specific requirements on mind (OTP assumed as default option, biometric hidden for casual users but available for advanced users, biometric flow easier to understand) so I'm not doing pull request on this, but feel free to pick anything you like :) What is done there:
This allow us to 'hide' some weird responses from biometric device UI or browser (for example about lack of usb key).
Things to do:
Javascript functions added: CheckBiometricsDevice()Asynchronous, check for bio device availability with PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable() and sets two global variables: CheckAutoTryOtp()Auto select and submit OTP method on select method screen if autotryotp cookie is present, then clears that cookie getCookie(cname)Get value from cookie named cname SkipBiometricDuringRegistration()Allow to auto ignore&submit biometric device during user registration when biometric method is not required. Javascript functions modified:
|
Beta Was this translation helpful? Give feedback.
-
Hi @redhook62, |
Beta Was this translation helpful? Give feedback.
-
Hi @apr-un First of all, I wanted to thank you for the efforts you have made (Translation and correction of bugs) I did install your version today, but I have a lot of comments.
The easiest way is to explain to me what you want to do, it can be interesting. however, I think that in view of the evolution of the protocol, and also for security reasons, that it is better to let browsers or operating systems display what comes from CATP. So I suggest you review this in September at a relaxed pace. regards And once again thank you for your investment. |
Beta Was this translation helpful? Give feedback.
-
Hi @redhook62 I've started workind on custom adapter version of my biometric changes. I've created new project (CustomAdapterPresentation) in Neos solution. Unfortunetly then I get many errors about lack of access to internal functions of Multifactor and Common projects, but I was able to fix them by adding to both project properties\assembly.cs: Now project will compile, but I've got two new problems:
Also I see that there is inconsistency between documentation and code - according to https://github.com/neos-sdi/adfsmfa/wiki/05-General-Settings this property should contain fully qualified CLASS name inherited from BasePresentation, whereas code looks for fully qualified ASSEMBLY name and then inside that assembly looks for type which inherit from BaseMFAPresentation ? Regards |
Beta Was this translation helpful? Give feedback.
-
hi @apr-un I admit that I did not test this configuration with another signature, it was partly indicated during the implementation of the release ... For the configuration, yes, the documentation was wrong, it has just been modified, the methods of BaseMFAPresentation can be overloaded or replaced. For visibility, on your own build you can do whatever you want, then two possibilities:
For Wix, either we do the build and there is no problem. |
Beta Was this translation helpful? Give feedback.
-
Hi @apr-un To change some properties on MFAConfig or others : $c = Get-MFAConfig
$c.AdapterPresentationImplementation = "Your fully named class BaseMFAPresentation"
$c.UIKind = Custom
Set-MFAConfig = $c
Set-MFAThemeMode -UIKind Custom -Theme MyThemeIfAny For the accessibility of class, i think that we are going to put a public wrapper class. regards |
Beta Was this translation helpful? Give feedback.
-
Hi @apr-un I will ship tomorrow a new version 3.1.2109.1 The component version is included in the installation as well as in the examples. regards |
Beta Was this translation helpful? Give feedback.
-
Hi, @apr-un Take a look at the latest build (3.1.2110.0/1 Provider and Resources are well assigned the the instance created. regards |
Beta Was this translation helpful? Give feedback.
-
Hi @redhook62 thanks from reply, I'll try to merge new version with my code. Let me repeat my main problem:
Alternative question: where should I place code to load my resources (if not in the constructors)? Regards |
Beta Was this translation helpful? Give feedback.
-
Hi, @apr-un What types of resources are you talking about? the embedded resources are loaded automatically by the .Net Framework, if you have correctly compiled the satellite assemblies. For the resources and the context, these parameters are passed after the constructor. Another point, the javascript, if it is not generated by the presentation but with external script, it must be registered in the WebConfig of adfs so that the execution of the code is trusted. regards |
Beta Was this translation helpful? Give feedback.
-
Hi, @apr-un Ok, it's a little clearer now. However, if we make the modification for the call of a constructor, it must be done for all the constructors. Next, for resources I'll have a look, but changing that in the constructor of the external component doesn't appeal to me. regards |
Beta Was this translation helpful? Give feedback.
-
Yep, all calls to LoadCustomAdapterPresentation from AdapterPresentation constructors should be modified and of course definition of this functionalso should be modified so it can accept the same parameters like its "parent constructor method". I think it will be easier to just prepare 5 overloads for LoadCustomAdapterPresentation and call them from corresponding constructors:
than prepare one definition of this function with all params and conditionally call Activator.CreateInstance with different values :) Could You explain what do You mean by: I'm calling Your's base constructors in mine and Yours resource are loaded normally, then my resources are added (eventually overwriting some translations if I need that). Regards |
Beta Was this translation helpful? Give feedback.
-
Happy New Year to you and your family. After reading your post, First, you can limit the browsers that can use biometrics since version 3.1.2112.0. On what you propose, there are interesting ideas.
which we will not implement.
We will implement these changes during the month of February or March. regards |
Beta Was this translation helpful? Give feedback.
-
@redhook62 thanks for response :) Just to be sure if I understand correctly - which options will be implemented in february/march ? Regards |
Beta Was this translation helpful? Give feedback.
-
Hi @apr-un What we will implement by March or later...
What we won't do.
PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable() can only detect build-in authenticators (Windows Hello, FaceID, TouchID or Android Device), so for example, as your screen shots show, FireFox (which only supports external usb devices such as Yubico key) will invariably return false on this call. Many things can also occur due to the configuration of WebAuthN: Authenticator Attachment, Attestation Conveyance Preference, etc... regards |
Beta Was this translation helpful? Give feedback.
-
Hi @apr-un I confirm one thing So, with your example code, Yubico keys are systematically excluded. So with Firefox your users will always get the message asking them to insert their Key... and they will always have to cancel to continue. We will remove this test and make sure window.PublicKeyCredential is not undefined so that it can continue to work in all cases. with "Platform" devices and "Cross-Platform" devices (USB, NFC, Bluetooth) regards |
Beta Was this translation helpful? Give feedback.
-
No, I myself have several Ybico keys, on a server that does not have Windows Hello, or on a Linux, with this javascript code it does not work whether for Firefox (I have version 96.0.3) or for Edge Chromium, or Chrome). The name of the method clearly indicates its "Platform" scope, the Yubikco keys are "Cross-Platform". So for me it is not acceptable. Then, if you want to move forward, let me know your vision, taking my remarks into account. regards |
Beta Was this translation helpful? Give feedback.
-
I did test done on Ubuntu 20.04.3 LTS with Firefox 96.0 and function works there (it exists and returns false). I think that You assume that "false" value means it is not working, but I explained before that its ok (js just can't detect authenticators). I hope that clarify my point o view, I also add that page with script if You want to check it yourself on linux or other system. Regards |
Beta Was this translation helpful? Give feedback.
-
Hi,@apr-un Before answering you the other day, I obviously tested the JS code of your page, the one recommended by different sites. In the last release, I removed this test, because it works under Windows 10/11 with Windows Hello if it is configured. On machines that do not have Windows Hello, or any Framework (Windows Server by default), the result is negative with all Browsers... in this case there is no built-in Authenticator. however without this JS code or ignoring the result, connecting with a Yubikey works (cross platform). It is therefore a question of testing with this JS code only the Built-in authenticators. Under Windows Server ou a Virtual Machine I would add Chrome on Android with a Yubikey in USB or NFC, the result is also negative. So, why put this code, if it's to ignore it ? And here we come to the bottom of the matter, with the MDS the solution supports more than a hundred devices (for example SafeNet, or HID Smart Card or NFC solutions). What I could do:
You can make your own interface (Custom Presentation), I have made many modifications to allow you to do this. Understand my position. this extension of MFA is a security extension, often in companies imposed by the Head of Security (RSSI), users must comply with the security constraints imposed. I can assure you that in a lot of companies it is not as simple as ADFSMFA offers. Last point, we make this product, in addition to our regular work and do not want to generate a mass of support requests or issues for people who do not have the same vision as you. (like me...) I think the best solution is for you to develop your own UI. regards redhook |
Beta Was this translation helpful? Give feedback.
-
Hi @redhook62 In mean time I did what You suggested. I have one project (one dll) as custom presentation adapter which is not a part of main MFA solution (has another signature) but has references to MFA dlls (not version specific). This wasn't possible when we started talking year ago, so big thank You :) I tried to configure MFA in powershell to use this new project for presentation, but somehow its still using my old version (I did restarts, no errors in logs):
Should it work? Or the keys needs to be the same? In summary - what I want to achieve:
Regards |
Beta Was this translation helpful? Give feedback.
-
Hi @apr-un Great News ! No, the signing keys can be different. You can post your solution (sources) in pull-requests.
regards |
Beta Was this translation helpful? Give feedback.
-
Hi @redhook62 I was testing versions 2203.0(my signature) and 2204.0(neos) with my custom adapter and I found two problems ;)
Old versions of these names were written in ADFS configuration database (!). After restart, new MFA server configuration files (system.db, config.db) were not created due to weird error in logs (Object reference not set to an instance of an object). After debugging on my version of MFA files it looks like this 'null reference' is realy caused by deserialization error in XML file ( new OTPWizardOptions doesn't have valid value for NoMicrosoftAuthenticator ). I fixed that by manually renaming these options to correct values in exported configuration file (xml) and importing it back using ADFS commands (Export-AdfsAuthenticationProviderConfigurationData / Import-AdfsAuthenticationProviderConfigurationData ), So in conclusion:
In event log I saw errors: I think these errors were caused by combination of:
I did reset these parameters to default values or in case of ADFA password to correct value, errors are gone. But still, during login I have another 'Null reference' error - this time from This function has try/catch block which hides real error:
In event viewer in application log I see: In ADFS log i see: These information are not helping :) Is it possible to change ex.Message in Log.WriteEntry to ex.ToString so all info will be logged (maybe in other places too)? Of course I can reinstall my version of MFA with my signatures back (I already upgraded it to 2204.0) and debug it, but this will not help me with core problem (using my custom adapter + Neos MFA) Regards PS: sorry for wall of text again ;) Problem 2: |
Beta Was this translation helpful? Give feedback.
-
Hi, Attached is a build with the changes you requested (ex.Message -> ex.ToString()) regards please delete this file after |
Beta Was this translation helpful? Give feedback.
-
Hi @redhook62
Can You fix this, please? Regards |
Beta Was this translation helpful? Give feedback.
-
Hi @apr-un! Sorry for reactivating this issue, I would like to collaborate in this project, but I have problems compiling the solution. I've been several days behind this error and I need help! I have followed the steps of @redhook62 and the ones you indicate in this issue. But when compiling, I get the following error:
Do you know what it can be? Thank you very much in advance, Regards |
Beta Was this translation helpful? Give feedback.
-
Hi @jredondo18 I remind you that the .Net 4.7.2 framework is a prerequisite. On the other hand, the System.Windows.Forms.dll assembly is part of FW 4.7.2 and its signature is as follows :
Please check your references regards |
Beta Was this translation helpful? Give feedback.
-
Hi @redhook62
I prepared some enchancements to Biometric process (client side, mainly for better flow and user experience) and I want to share them. I had several problems with application building/compiling from source code, but at last I've managed to do it successfully on ADFS2019 in version v3.12105.0.
I had to do several adjustments to source code first, and I'm not really sure if I'was doing it correctly, so I'll try to describe my process. Please correct me where it should be done differently :)
I download code from this git repo and open it in VS(2019), and press Build - then I see over hundred errors (mainly due to lack of compiled dlls from wix project or missing pfx files). I have to retarget 'NativeResources' project to Windows SDK 10.0 (latest version), and PlatformVersion to v142 (vs2019).
I go to DataTypes project (because no dependency to other projects) and sign it with new password protected file with name 'Neos.IdentityServer'. This one project should build correctly now.
I go to every project which has missing pfx file and select the same file 'Neos.IdentityServer.pfx' from DataTypes project. Now few other projects should build.
Building solution is still not possible due to several errors looking like:
Friend access was granted by 'Neos.IdentityServer.MultiFactor.DataTypes, Version=3.0.0.0, Culture=neutral, PublicKeyToken=79dcaed57e22cca7', but the public key of the output assembly ('00240000048....009af5ac298231142f23f44c0') does not match that specified by the InternalsVisibleTo attribute in the granting assembly.
To fix that I use procedure from AssemblyInfo.cs to get corrected public key and public key token for my signature file (pfx)
In VS developer command line I go to folder with pfx file (in DataTypes project) and execute two commands:
sn -p Neos.IdentityServer.pfx Neos.IdentityServer.publickey
then
sn -tp Neos.IdentityServer.publickey
This should show publickey (in multiple lines) and token. Public key have to be copied in single line(!):
`Public key (hash algorithm: sha1):
00240000048000009400000006020... 298231142f23f44c0
Public key token is 79d...cca7`
Now I can use this generated public key(long line) and replace with it ALL public keys in lines with 'InternalsVisibleTo' in all AssemblyInfo.cs files (should be 19 lines and 5 files)
'Friend access...' errors are now gone, but this replacing looks improper to me :(
Maybe these values can be loaded from app settings or some resource?
Solution still cannot be build at this point, because I get error in SAS.Azure project:
'AuthenticationContext' does not contain a definition for 'AcquireToken' and no accessible extension method 'AcquireToken' accepting a first argument of type 'AuthenticationContext' could be found (are you missing a using directive or an assembly reference?)'
In short: Microsoft.IdentityServer.Aad.Sas.dll doesn't have this method, there is only AcquireTokenAsync method - maybe this is specific to ADFS 2019?
I can fix that by using async method:
var taskResult = this.authContext.AcquireTokenAsync(this._resourceUri, this._clientAssertion); authenticationResult = taskResult.Result;
Solution is still not building correctly because lack of config files in Deployment, Multifactor and NotificationHub projects.
In Deployment there must be CustomAction.config with specific content (I've spent some time to find it :) ):
In Multifactor and NotificationHub app.config files are probably not needed (I'm not sure what they can contain?) and can be excluded from solution.
Theres only one error now, lack of resgen.exe in project Console/SnapMMC.
This file can be copied from .NET tools - for example: C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8 Tools
Solution is now building successfully, but it won't work correctly after installation (wrong signatures of DLL)
Theres one last thing to change - hardcoded string
PublicKeyToken=175aa5ee756d2aa2
needs to be replaced with our own public key token79d...cca7
from step 6 (28 times in 8 files total). Again this looks improper - maybe these values can be loaded from app settings/resource?Successfully builded application adfsmfa.msi is located in 'Neos.IdentityServer 3.1\Neos.IdentityServer.MultiFactor.WixSetup\bin...'
To debug installed application on ADFS node I need to attach to processes Microsoft.IdentityServer.Hosts.exe and Neos.IdentityServer.MultiFactor.NotificationHub.exe
Please confirm if this process is correct, and then I will put new pull request with my changes to review.
Thanks
Beta Was this translation helpful? Give feedback.
All reactions