threat-dragon icon indicating copy to clipboard operation
threat-dragon copied to clipboard

Support Azure OAuth

Open lreading opened this issue 3 years ago • 14 comments

Describe what problem your feature request solves Add ability to use multiple passport connectors, such as Azure AD.

Describe the solution you'd like I'd like to add the Azure AD Passport connector for authentication/authorization. I think it would make the most sense to have this be enabled via configuration, and also allow for multiple passport strategies. For example: export PASSPORT_STRATEGIES="['github', 'azuread-openidconnect']" or export PASSPORT_STRATEGIES="['github']" This would open up the possibility of adding other passport strategies/connectors in the future.

Additional context Some questions I have:

  • Because Azure AD requires a bunch more config options, would it make sense to implement something like dotenv to make configuration a bit easier?
    • If so, should that be a separate feature / PR to keep the scope somewhat limited?
  • Does it make sense to restructure how the passport authentication is configured to make it easier to add other authentication types in the future?

I'm eager to add this functionality, but wanted to see if it's a desired feature, and what the appetite is as far as the size and scope of pull requests.

lreading avatar Apr 23 '21 16:04 lreading

Hello @lreading , yes, definitely an appetite for what you are describing. We are looking towards Threat Dragon 2.0 this year and expanding the ways to authenticate is central to this. We have a roadmap for version 2.0, and we would like various ways to authenticate - sort of similar to https://app.diagrams.net, as we are also wanting to use mxgraph.

Certainly will accept large pull requests, but try and break them up if you can, and the scope is the whole project - do not hold yourself back

Cheers, Jon

jgadsden avatar Apr 24 '21 06:04 jgadsden

@lreading I would normally assign this issue to you, but you have to be a contributor - so add some code and then we can get you on the contributors list. I have assigned it to myself for now

jgadsden avatar Apr 24 '21 06:04 jgadsden

I somehow missed the multi auth when I looked at the roadmap. :facepalm:

A few more questions:

  • For Azure, specifically, is there a preference for back-end storage of models, such as azure table storage?
  • Would you prefer this to target the main trunk or the 2.0 branch?
  • Do you want feature requests / PRs for individual items on the 2.0 roadmap, or do you plan to use a github project (I see one for 1.5)?
  • If targeting 2.0, would you like help refactoring to ES6 before adding additional code?

Thanks again!

lreading avatar Apr 24 '21 16:04 lreading

Ah, I thought I had put authentication into the roadmap, and when this issue was raised I realised I had missed it out. I then added it and that is why you did not see it before, sorry to have tripped you up.

Some answers as best as I can:

  • Myself I do not use Azure, so can not say about storage. @andk123 or @mike-goodwin do you have a preference?
  • target the main branch please, we can then add it in for version 1.4
  • version 2.0 is more of a technology change, forced by our jointjs issue, rather than changes in features
  • if we manage to put version 2.0 roadmap features into version 1.x then so much the better
  • we have a 1.5 and a 1.4 github projects for the other repos (core and desktop) so I have created a 1.4 github project for this repo
  • certainly refactoring to ES6 is appreciated

Hope this helps, many thanks for the contributions to the code base

jgadsden avatar Apr 25 '21 04:04 jgadsden

Hello, no preference in this case. I haven't worked with Azure either, but I would go with the simplest solution in this case (or the simplest for you). I assume azure table storage is well integrated with azure so you should go for it.

andk123 avatar Apr 25 '21 23:04 andk123

It looks like we will not have this in time for version 2.0, so moving it to version 2.x

jgadsden avatar Aug 31 '22 10:08 jgadsden

@jgadsden any chance this issue is up for contributors? My employer is currently looking at Threat Dragon as a possible threat modeling tool, and I'd love to help make AD authentication available if possible!

ppeters0502 avatar May 11 '23 20:05 ppeters0502

Hello @ppeters0502 , very much appreciated if you would like to take this on - can I assign this to you?

jgadsden avatar May 12 '23 05:05 jgadsden

Sure thing! I'll probably be sticking pretty close to how the github OAuth setup is, but if there's anything specific to AD I get stuck on, I'll be sure to call out!

ppeters0502 avatar May 12 '23 14:05 ppeters0502

Thanks @ppeters0502, I have assigned it to you and added it to the version 2.1 milestone Do not feel under pressure, if version 2.2 turns out to be better for you then that is great as well

jgadsden avatar May 12 '23 17:05 jgadsden

Thanks @jgadsden, I'm working on this now! Related question, threat dragon currently supports either saving models in a github repo, or (when using local mode) saving the model to the user's machine. If supporting Azure (either OAuth or Open ID), should we also support saving models via OneDrive? Or for simplicity's sake should we still require github credentials for configuration and stick with either saving models in github or locally?

ppeters0502 avatar May 19 '23 03:05 ppeters0502

Hello @ppeters0502 good to see progress on this, much appreciated, it has been wanted for a long time If you can save files via OneDrive then that would be really good, did you want to create a new issue specifically for this or implement within this one?

jgadsden avatar May 19 '23 06:05 jgadsden

So I re-read through the different comments on this issue and had missed that azure storage had been mentioned already. I agree with the earlier comments, this would probably actually be the simplest solution as far as enabling a non-github storage option for models. It could be as simple as a couple additional parameters in the config to designate an Azure storage account/location. Then if the user is using Azure AD for authentication/authorization, we check for Azure storage config info, and either attempt to save in Azure, or default to local storage if there are errors or if there is no Azure storage info provided.

ppeters0502 avatar May 19 '23 18:05 ppeters0502

Hello @ppeters0502 , yes sure, this makes sense I myself have very little knowledge/understanding of Azure so over to you on this one (shame on me but over the years I have avoided anything Microsoft) For sure Azure support is a feature that I am sure a lot of the community will want and use

jgadsden avatar May 20 '23 04:05 jgadsden