Community
Participate
Working Groups
For example querying a private phpBB or vBulletin forum is not possible because authentication is not implemented. To support adding login implementation to templates, maybe the extension point could optionally include a reference to a class that implements a certain interface. I imagine that making this very generic is quite a task, but it would be good to support at least the typical login scenario that asks for username & password and uses cookies for state -- at least phpBB and vBulletin usually work this way. I am willing to help with this, but I will be unavailable for the next 7 or 8 days. A working HttpClient cookie-based login implementation for phpBB and vBulletin can be found here: http://bbapi.tigris.org/source/browse/bbapi/trunk/ , in the org.tigris.bbapi.phpbb and org.tigris.bbapi.vbulletin plugins. vBulletin ( look in PHPBB.login(IBBCredentials) and the same method in the VBulletin class )
Doesn't entering the credentials via the web browser work for the duration of the Eclipse session? That has been my experience using Bugzilla and Jira in the web mode. Note that we're starting to upt some support for HttpClient connections in mylar.internal.tasks.core.WebClientUtil
(In reply to comment #1) > Doesn't entering the credentials via the web browser work for the duration of > the Eclipse session? That has been my experience using Bugzilla and Jira in > the web mode. Yes, but those are not used when fetching query results.
(In reply to comment #2) > (In reply to comment #1) > > Doesn't entering the credentials via the web browser work for the duration of > > the Eclipse session? That has been my experience using Bugzilla and Jira in > > the web mode. > > Yes, but those are not used when fetching query results. > entering credentials for sourceforge does not work for me, linux firefox 1.5. but that might be a linux browser problem, as it sometimes causes Eclipse to crash!
(In reply to comment #3) > entering credentials for sourceforge does not work for me, linux firefox 1.5. > but that might be a linux browser problem, as it sometimes causes Eclipse to > crash! Can you please clarify where you entering credentials? User and password from repository dialog are not used right now, but Mylar does not allow to remove these fields from there for repository connector. However, browser could ask for credentials when entering some pages and is completely in the browser. Though as far as I recall, SF is using form-based auth, which is not needed to run queries, only when adding or editing issues.
Ok, I finally have actual need for this and I'm thinking it could be done like this: allow to specify via UI 1) a login URL 2) OPTIONAL: whether it should be a GET or POST request (just doing a POST is probably enough for all cases in reality) 3) specify all GET/POST parameters as name/value pairs (the values could use string replace to replace %username% and %password% or something like that) 4) OPTIONAL: specify separately which parameter name is used for username and password Then just remember the cookies. I could use this to experiment using Mylar with ChangeLogic manually for about two weeks with another developer. The information we gather this way should be shared with the ChangeLogic developers and hopefully we will help them decide to create a ChangeLogic integration :) Adding Karel Kravik to cc. -- Karel, is the above list enough to login to ChangeLogic? Or maybe logging in with an activation key is possible? (explanation: after the login request, another request would be made to query the task list and parse it with regular expressions).
Created attachment 52731 [details] Naive patch that just adds a loginUrl This patch just adds a loginUrl. I wasn't able to test this yet due to the issue with PDE and mylar.context.core, but I'll create a proper self-hosting workspace for mylar and test it then.
Created attachment 52732 [details] mylar/context/zip
Actually, this was not enough for ChangeLogic. The login form uses some kind of security token, so the possible generic steps to log in using GET is: -------- If (tokenPattern) { html = get(loginPageURL); token = parsePageForToken(html, tokenPattern); loginURL = loginURL.replace("%TOKEN%", token); } loginURL = loginURL.replace("%USERNAME%", username); loginURL = loginURL.replace("%PASSWORD%", password); get(loginURL); ... continue as usual ... -------- In the UI, this would add "Login page URL", "Login page token regexp", "Login URL", and the loginURL would contain variables TOKEN, USERNAME, PASSWORD. What do you think, Eugene? I will update the patch to reflect this soon.
Created attachment 52749 [details] New patch, according to the logic that allows for parsing login token This is the new patch that adds login page url, login token regexp. It currently seems to work fine with changelogic. Sorry, but the context is not updated atm.
Eugene: since this bug belongs to you please request for me to apply any patch that you would like applied and I will review.
Created attachment 52821 [details] Path with fixed wizard page initialization The previous patch didn't initialize the new fields in the wizard page if you were editing an existing query.
Created attachment 52824 [details] Patch with fixed externalization Fixed externalization, previously the value of Login URL was externalized instead of Login Page URL's value.
(In reply to comment #10) > Eugene: since this bug belongs to you please request for me to apply any patch > that you would like applied and I will review. Will look at it next week.
I'm thinking that putting the password in the URL is a bad security blooper, so the POST parameters mentioned in comment #5 really should be added to this model.
Created attachment 52838 [details] This patch adds template support This patch adds the possibility to specifiy the login url and login token regexp in the templates. But I didn't add the "Login page url" as a template attribute, but used the Repository URL to fill that field.
Erkki, why do you have login parameters at the query level? Shouldn't all these params be in the repository settings?
Hmm, now that I think of it, you are right. Should be at the repo level. I think my logic was that nothing besides the query requires that kind of login.
Eugene, I really like the new UI for the web repositories. How do you think the login parameters could be added to that? If just replacing the username/password in the URL, that would be easier, but I don't think it should be done like that -- it was just a temporary solution for me. So another parameter table is probably necessary for a POST login. I suggest to add option 1) to the hidden advanced section: Login token pattern (that would be read from the main repository page, but maybe in some rare cases where both public and privileged access is possible there is a need to read that token from a different page... that case should probably not dealt with prematurely though, as I did in the original patch) Login URL A table with POST parameters for the login. option 2) add the same items in a new section "Login configuration" below the "Advanced configuration" that is also hidden by default option 3) add a button to open a new dialog with the login options. I personally like option #2 the most.
Or actually, as it seems the ParametersEditor would work better on a different page, why not have an "Anonymous Access" checkbox checked by default as in the Bugzilla and Trac repository settings and in case it is unchecked, make the Next button go to a new advanced login settings page? Hmm... on the other hand, the advanced page should not be shown at all in the most common cases. I think in that case both "Next" and "Finish" could be visible and "Next" would go to the advanced login configuration page while Finish would finish immediately.
Created attachment 54090 [details] Rough preview of the proposed UI additions This is roughly how the new Web Repository settings page would look after adding the login configuration.
Created attachment 54115 [details] This patch adds all the login section to the new UI This patch is definitely not final, but I'd like to get feedback before I start refining it, if possible.
Created attachment 54116 [details] mylar/context/zip
Created attachment 54122 [details] mylar/context/zip I seem to have accidentally corrupted the previously attached context
Created attachment 54124 [details] Patch to improve the UI of the expert login configuration section This also creates two Sections inside the login configuration because the login token stuff is probably not needed for most of the repositories out there (I think). Notes about the UI: 1) there are some layout issues remaining (the sections are too wide) 2) CCombo should be used instead of Combo, but that needs a CComboViewer 3) the parameter entry is done directly in the table instead of a popup dialog as in the ParametersEditor; it has no validation yet
Created attachment 54125 [details] mylar/context/zip
Created attachment 54126 [details] Bugfixes and new ChangeLogic templates to the previous patch 1) several bugs fixed that the previous patch created 2) there are now 2 ChangeLogic templates: 2a) anonymous (default server changelogic.araneaframework.org) -- this allows to view public tasks anonymously. The query pattern is not perfect -- for some tasks the status is retrieved instead of the description 2b) with POST login (default server also changelogic.araneaframework.org -- it allows users to sign up) -- this worked for me with 3 different changelogic instances.
Created attachment 54128 [details] Updated screensot of the Login Configuration UI
This looks great Erkki. I think that it just needs a bit of UI simplification and we can apply the patch. Where is the "Login Tken Pattern" coming from? Should it be part of the template? Eugene, could you take a look and verify it? 1) Remove the folding sections, because they add UI complexity, and if a user is looking at this they should see it all. Replace them with check boxes that cause the items below them to enable. As an example take a look at the Proxy Settings section of the Bugzilla repository properties page, just checked into CVS a moment ago. 2) Put URL on the same line as Method to save a line, with Method going first. 3) "Advanced Configuration" -> "Template Configuration (Advanced)" 4) "Login Configuration (Expert)" -> "Login Configuration (Advanced)" 5) To get rid of the white call setBackground on the composite with the container's background.
(In reply to comment #28) > Should it be part of the template? Eugene, could you take a look and verify it? Yes, it is definitely should be coming from template. I haven't look at the code yet, but have few comments in regards to UI and some implementation details. First of all this folding idea (even for original advanced section) is becoming a real mess. The size of expanded UI is too large to sit under the fold. I think it all should be moved to the separate page (or a popup edit dialog in worst case). Also, we don't need two property tables. There should be only one! > ...As an example take a look at the Proxy > Settings section of the Bugzilla repository properties page, just checked into > CVS a moment ago. Hmm. Why it only appear for Bugzilla and not for other connectors? It is also very disappointing that you used folded sections instead of multiple pages. :-( > 2) Put URL on the same line as Method to save a line, with Method going first. > 3) "Advanced Configuration" -> "Template Configuration (Advanced)" > 4) "Login Configuration (Expert)" -> "Login Configuration (Advanced)" Actually, I believe there should be just one "advanced" section. There is no point to separate this stuff. For example, login token can be used in other urls. So, they are quite related. > 5) To get rid of the white call setBackground on the composite with the > container's background. That one I actually left white intentionally, to separate from the other UI. Either way, I'd like to take my own pass to the UI. Ie remove all the borders and align the widgets properly
(In reply to comment #29) > Yes, it is definitely should be coming from template. Yes, and an example is present in the ChangeLogic templates also included in this patch. > a real mess. The size of expanded UI is too large to sit under the fold. I > think it all should be moved to the separate page (or a popup edit dialog in > worst case). I agree that a separate page would be good. But should it be accessible by the "Next" button or in some other way? > Also, we don't need two property tables. There should be only one! Do you mean that there should be only one table where you enter properties (1) or that there should be only one implementation of a table where you enter key/value pairs (2). If it is 1) then I disagree -- the login request parameters should be hidden by default unlike the template parameters. If it's 2) then I agree -- the whole restui package could probably be replaced with something better, I did it the way it is because it was easy to develop a prototype that way in Visual Editor. Also, I think to enter the parameters directly in the table is better than a pop-up dialog (but maybe harder to do validation? -- haven't tried that in a table yet). > > 2) Put URL on the same line as Method to save a line, with Method going first. Moving the method and URL to one line sounds good. > Either way, I'd like to take my own pass to the UI. Ie remove all the borders > and align the widgets properly Ok.
(In reply to comment #30) > I agree that a separate page would be good. But should it be accessible by the > "Next" button or in some other way? Technically you not suppose to have "Next" buttons on properties dialogs. Those buttons are used for wizards. See Team/CVS repository properties. > > Also, we don't need two property tables. There should be only one! > > Do you mean that there should be only one table where you enter properties (1) > or that there should be only one implementation of a table where you enter > key/value pairs (2). > > If it is 1) then I disagree -- the login request parameters should be hidden by > default unlike the template parameters. You don't have anything to hide. More over, there is no point to map username to just ${userId} variable, so as other remappings you have used. Regardless, two property tables on the same dialog are really confusing and really make no sense.
(In reply to comment #31) > Technically you not suppose to have "Next" buttons on properties dialogs. Those > buttons are used for wizards. See Team/CVS repository properties. Ok, I see what you mean. > > If it is 1) then I disagree -- the login request parameters should be hidden by > > default unlike the template parameters. > > You don't have anything to hide. More over, there is no point to map username > to just ${userId} variable, so as other remappings you have used. > Regardless, two property tables on the same dialog are really confusing and > really make no sense. But I don't really see how you could present any differently the info that needs to be sent with the POST login request. You could call them repository parameters, but they aren't -- they won't be used as variables, they are used only in that login HTTP request and nowhere else. And you can't send the other parameters with the login request, so to put them in one table, you'd have to somehow separate them with a prefix or add another column where you select which kind of parameter it is. In ChangeLogic's case, I could actually use a GET method and put the whole POST Parameters table in the query, but that would be a more insecure way to send them. The mapping for ${userId} etc. is very much necessary if you want to use the standard repository username/password parameters to send with the login request and not enter them twice. All this works exactly the same way as what you did with introducing the parameters/variables -- you enter somewhat human-readable parameters in one table and the other more advanced fields make use of those variables.
(In reply to comment #32) > But I don't really see how you could present any differently the info that > needs to be sent with the POST login request. Why exactly you need to present them differently? Can't you just put them into the post url? > You could call them repository > parameters, but they aren't -- they won't be used as variables, they are used > only in that login HTTP request and nowhere else. And you can't send the other > parameters with the login request, so to put them in one table, you'd have to > somehow separate them with a prefix or add another column where you select > which kind of parameter it is. Parameters only sent somewhere if they are actually used in the urls. I see what you meant them for now, but if passing those in the url works for you, it would make UI less cluttered, especially with currently used folding... Or even better, parse that login url and convert params from the url into post parameters. This way it is even easier to store in templates. > In ChangeLogic's case, I could actually use a GET method and put the whole POST > Parameters table in the query, but that would be a more insecure way to send > them. Don't be silly. There is nothing secure about post. :-) If you really care about security, use https. > The mapping for ${userId} etc. is very much necessary if you want to use the > standard repository username/password parameters to send with the login request > and not enter them twice. I know. That is why I added ${userId} variable in first place.
Yes, POST is not secure by itself, but are you sure that in HTTPS there is no difference between how the body of a request and the URL are treated? POST parameters are sent in the request body, not in the URL. Hmm... maybe this difference between URL / body is a misconception I have... I'm forgetting that this is not used in a browser where a URL is more insecure because it's usually always visible. But what about caching, proxies etc. Do they not make a difference between POST/GET (or URL and body). I must admit that I'm no expert on this and I'm not 100% sure on any of it. Parsing the URL into post parameters is one possibility, but there might be some cases that expect some parameters in the URL and some as POST parameters (If I'm not mistaken, phpBB or vBulletin did that somewhere).
Actually, I checked phpBB & vBulletin login impl. in BBAPI and neither require any URL parameters besides POST parameters. The probability of some application requiring that should be quite low. So I think parsing the URL into post parameters would be ok and the table could be dropped.
Great. Parsing login url it is. :-)
Created attachment 54277 [details] Updated patch with simplified UI 1) The login request now fits on one line. Maybe the 'restui' package should be removed now and the code embedded in the settings page compilation unit? 2) no more special parameters -- as discussed, for POST method the URL query is now parsed into the POST method parameters. The parsing is not perfect, though. For example, the parameters are evaluated before the URL parsing so if the password/username contains &, it will be seen as separator. Not entirely sure yet how it's best to keep this URL parsing simple while allowing special characters. 3) synchronized with latest CVS changes -- mostly it was that there seemed to be some differently formatted code in CVS Note: since for the parameter evaluation, the fetchResource method now needs the repository as a parameter, maybe some other parameters from the method could be dropped.
I can't apply patch because of conflicts in WebQueryWizardPage and WebRepositoryConnector classes. Are you sure you have latest code? PS: when substituting parameters we need to use http encoder. There is one in Mylar core classes...
Created attachment 54288 [details] Fixed patch, now fetchResource() has less parameters Conflicts resolved, had to manually update the files and paste over the modified contents. In WebRepositoryConnector: 1) the fetchResource() now doesn't take as many parameters, as the TaskRepository is given as a parameter. 2) did an extract method refactoring: refreshIfNeeded() Which class is that HTTP encoder you were talking about? Maybe you could do that part yourself?
FYI. With this patch applied I got an NPE for existing query: java.lang.NullPointerException at org.eclipse.mylar.internal.tasks.web.WebRepositoryConnector.evaluateParams(WebRepositoryConnector.java:432) at org.eclipse.mylar.internal.tasks.web.WebRepositoryConnector.evaluateParams(WebRepositoryConnector.java:415) at org.eclipse.mylar.internal.tasks.web.WebRepositoryConnector.performQuery(WebRepositoryConnector.java:177) at org.eclipse.mylar.tasks.ui.SynchronizeQueryJob.run(SynchronizeQueryJob.java:74) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:58)
(In reply to comment #40) > FYI. With this patch applied I got an NPE for existing query: That seems to be because of null login form URL. Not sure why I haven't seen this error. I think perhaps somewhere null parameters got replaced with empty strings, but maybe your existing query still had null? Were you expecting that evaluateParams(String, Map) would never be called for a null value? I wasn't aware of that. PS. I don't want to create yet another patch in this bug, but on line 317, the refreshIfNeeded() call should assign to get, like this: get = refreshIfNeeded(loginFormUrl, client, get);
Created attachment 54299 [details] Redesigned UI layout This is my take on this UI. Damn it. It is getting more obscured then I initially thought. I now wonder if this ChangeLogic is the only issue tracker that require this token thing...
I think it would really be better on a separate properties page and in Groups. But then the whole properties dialog should be redesigned in every Mylar connector, I think? Currently it's designed like a Wizard, but functions more like those CVS/SVN properties pages, like you mentioned somewhere above. I personally think doing this would be good, but probably too much work for you guys right now?
Yes, redesigning the Repository Properties page to one of the other options (tabbed, or multi-page properties) can not happen for 1.0. There are tricky problems such as having the "Validate Settings" button in a sensible place where you can see all the relevant settings. Also, the folding approach provides a good forcing function for making the UI dense, as Eugene seems to have done a very good job of in his last iteration. Eugene: Let me know when you guys have the patch in a state where you want me to apply it. Also, is ChangeLogic really the only tracker that this will support? That's not great, bug I guess it's OK if we're reasonably confident that this mechanism could be useful for other web based repositories that we will support down the road.
(In reply to comment #44) > Eugene: Let me know when you guys have the patch in a state where you want me > to apply it. Ok then, I'll finish UI changes I started and will cleanup Erkki's code (ie remove printStackTrace() and some other stuff). > Also, is ChangeLogic really the only tracker that this will support? That's not > great, bug I guess it's OK if we're reasonably confident that this mechanism > could be useful for other web based repositories that we will support down the > road. Sorry, I meant that that logging in should work on others repositories. Though I yet have to check how Erkki implemented passing session between requests. What is specific to ChangeLogic is that login token id and hence an extra request to the server to fetch it. As I said, I don't recall anyone else doing something like that, but I can be wrong.
Session is currently not persisted between requests (would this be useful? personally I didn't consider that to be important). To persist the session, the HttpClient or it's cookie state should be cached. I guess there is even some kind of possibility of sharing the session with the internal browser? I have seen this token usage in other web applications, but I'm not sure if any issue repositories use that. vBulletin or phpBB I think uses something similar during posting messages, but as I remember it isn't mandatory.
Created attachment 54456 [details] Reworked patch with login support and new templates Here we are. I am sorry to say that there is very little code left from the original patch because I rewrote most of the resource fetching code. Since there is no public CodeLogic repo with auth, I've used OTRS as a driver for this. Mik, can you please review and apply this patch. WebRepositoryConnectorTest may flicker on OTRS repository, because tasks are up and down. So, this patch adds 3 new templates: OTRS, CodeLogic and CodeBeamer. Erkki, I used one template for both repositories. Please check if it work for you once Mik will apply this patch. Then, if you need additional fixes, please make sure that WebRepositoryConnectorTest is working. Thanks.
Created attachment 54457 [details] mylar/context/zip
Folks, if this patch (finally) gets applied on the next hours, is it possible to release a dev-build to try this feature? I have not external CVS access here, but I'd like to experiment with it.
Also, is it complicated to add Mantis login support out of the box too, Eugene?
Patch applied. I did a quick review but did not validate. Got this into dev build 0.9.0.v20061124-0730 which is now uploading so it should be available in 15-30 minutes. Check it out and post any updates. Note that we messed up the Task Repositories page for you some and will clean that up today. I think for now we'll make the Character encoding fold since most people don't need to see that.
I was just going to make encoding a single dorp down, or checkbox (Default Encoding) and dropdown...
> Here we are. I am sorry to say that there is very little code left from the > original patch because I rewrote most of the resource fetching code. I don't mind :) >Erkki, I used one template for both repositories. Please check if it work for >you once Mik will apply this patch. Then, if you need additional fixes, please >make sure that WebRepositoryConnectorTest is working. Thanks. It doesn't work for me at the moment (with the private repo), I will take a look at it (and the new code) later when I get home.
Eugene: the single drop down sounds good, and if you end up doing this just try to put "Default (UTF-8)" as the first entry because that list is soooo huge and if someone clicks this accidentally they will never find the default again. It's OK if it's duplicated and shows up as (UTF-8) later.
(In reply to comment #53) > It doesn't work for me at the moment (with the private repo), I will take a > look at it (and the new code) later when I get home. Note that there is a bug with POST method authentication and redirect handling in the applied patch. See bug#165852.
(In reply to comment #47) > Erkki, I used one template for both repositories. Please check if it work for you once Mik will apply this patch. Then, if you need additional fixes, please make sure that WebRepositoryConnectorTest is working. Thanks. Merging the patterns doesn't work well because the private task list page differs from the public one. Perhaps the private template should not be part of Mylar (unless you accept having two Changelogic templates), and I could contribute it as a patch or separate jarred plugin attached to the Changelogic enh. request (bug 159346)?
Sorry, my bad, it seems you succesfully managed to make the pattern support both pages, but what was causing me trouble was the problem mentioned by Willian. Also, another thing: not only HTTP 302 should indicate that the new URL be read from the Location header, but also: 301,302,303,307 Also, with 300, "user agents MAY use the Location field value for automatic redirection" This is for HTTP 1.1, don't know about 1.0 I specifically need it to work with 301, because my server redirects the login page from something like cvs.lan/changelogic to cvs.lan/changelogic/ with "301 Moved Permanently"
(In reply to comment #57) > Also, another thing: not only HTTP 302 should indicate that the new URL be read from the Location header, but also: > 301,302,303,307 > Also, with 300, "user agents MAY use the Location field value for automatic redirection" > This is for HTTP 1.1, don't know about 1.0 But actually, why do this manually and set method.setFollowRedirects(false), when this could be done by HttpClient instead? It looks like this is how HttpClient does it internally: switch (method.getStatusCode()) { case HttpStatus.SC_MOVED_TEMPORARILY: case HttpStatus.SC_MOVED_PERMANENTLY: case HttpStatus.SC_SEE_OTHER: case HttpStatus.SC_TEMPORARY_REDIRECT: LOG.debug("Redirect required"); if (method.getFollowRedirects()) { return true; } else { LOG.info("Redirect requested but followRedirects is " + "disabled"); return false; } default: return false; } //end of switch
(In reply to comment #58) > But actually, why do this manually and set method.setFollowRedirects(false), > when this could be done by HttpClient instead? It looks like this is how > HttpClient does it internally: I originally tried that, but was getting circular redirect error from http client. No idea why.
(In reply to comment #59) > (In reply to comment #58) > > But actually, why do this manually and set method.setFollowRedirects(false), > > when this could be done by HttpClient instead? It looks like this is how > > HttpClient does it internally: > > I originally tried that, but was getting circular redirect error from http > client. No idea why. > Also, the setFollowRedirects(...) does not work with POST method.
Erkki, I updated the patch on bug#165852 regarding the error codes.
Anything left for me to apply before 1.0 here?
(In reply to comment #62) > Anything left for me to apply before 1.0 here? > I think this is ok. The patch in bug#165852 was applied too, so it is working with POST authentication (Erkki, is this working for you now?).
(In reply to comment #62) > Anything left for me to apply before 1.0 here? I don't have anything right now. We should track further issues in a separate reports.
Sounds right to me.
